[PATCH] D54222: [clang-tidy] Add a check to detect returning static locals in public headers

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

rsmith added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-problematic-statics.rst:7-8
+Detects functions defined in headers that return the address of a static
+local variable. These are not guaranteed to have the same addresss across
+shared libraries and could be problematic for plugins.
+

The C++ language rules do guarantee that such static locals have the same 
address throughout the entire program. Should this warning check the symbol 
visibility / `dllexport`edness? People are going to learn the wrong thing if we 
give them this diagnostic without any further explanation.

If the concern is plugins loaded with `RTLD_LOCAL`, sharing C++ interfaces 
across an `RTLD_LOCAL` boundary is pretty fundamentally broken, and this is far 
from the only way in which it goes wrong. It might be better to warn on all 
non-`RTLD_GLOBAL` calls to `dlopen` from C++ code...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54222

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

[PATCH] D87962: [clang] Change the multi-character character constants from extension to implementation-defined.

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

rsmith added a comment.

In D87962#2306043 , @aaron.ballman 
wrote:

> That doesn't sound like the right approach to me -- Remarks are usually for 
> reporting backend decision-making to the frontend for things like 
> optimization passes.

To be clear: that's how they happen to most visibly be used, but the more 
general idea is that Remarks are for purely informational messages. One test 
for whether a diagnostic could reasonably be a remark is: can we imagine anyone 
ever reasonably wanting to promote it to an error as part of the flags for some 
project? If so, then use of a remark is inappropriate. That's certainly the 
case here: it's entirely reasonable to want to be able to reject use of 
non-portable functionality such as this. So I agree, this should not be a 
remark.

> Btw, it looks like when you generated this patch, it was generated against 
> the previous diff and not trunk, so the diff view is misleading. When you 
> update the patch, can you be sure to diff against the trunk?

Please also ensure you upload a diff with full context 
.


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

https://reviews.llvm.org/D87962

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

[PATCH] D88680: Add ability to turn off -fpch-instantiate-templates in clang-cl

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

> I tried pretty hard to get a small repro for these failures, but couldn't.

Can you get at least some testcase, even if not small? You can use -E 
-frewrite-includes to create a single large file from all the input. Although 
the patch looks fine to me as such, I consider it a workaround. The reason for 
-fpch-instantiate-templates being the default for clang-cl is that MSVC does 
some kind of template instantiation for PCHs too (although seeing your email 
you probably know more than I do). So preferably the core problem should be 
fixed.

In D88680#2307564 , @rnk wrote:

> I think the flag was originally intended to be an internal -cc1 flag not 
> exposed to users.

No, it's intentionally exposed in the gcc-compatible driver, because there the 
option is not the default because of corner cases if the PCH is not 
self-contained. But since MSVC requires PCHs to be self-contained, it was 
deemed safe to always enable it in clang-cl. And, as said above, I still think 
it generally should.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88680

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

[PATCH] D63640: [clang] Improve Serialization/Imporing of APValues

[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.

Looks great, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63640

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

lanza requested review of this revision.
lanza marked an inline comment as done.
lanza added inline comments.



Comment at: clang/lib/CodeGen/CGObjC.cpp:490
+  llvm::UniqueVector FoundProtocols;
+  std::set DeclaredProtocols;
+

rjmccall wrote:
> You should use llvm::DenseSet for this.  But in general there are more sets 
> here than I think you really need, and you're building them eagerly when you 
> don't have to.  I would suggest:
> 
> 1. Walk the list of declared protocols, adding the runtime protocols to the 
> main result list and the first runtime protocols implied by the non-runtime 
> protocols to a different list.
> 
> 2. If there are any protocols in the first-implied list (which is unlikely), 
> build the implied-protocols set for all the protocols in the main list 
> (inclusive of the protocols there) and the first-implied list (exclusive).  
> It would be totally reasonable to add a method to make this easier: `void 
> ObjCProtocolDecl::getImpliedProtocols(llvm::DenseSet 
> &) const;`
> 
> 3. Add any protocols that are in the first-implied list but not the implied 
> set back to the main list.
> 
> Also, protocols can be redeclared, so you should make sure to map to the 
> canonical decl before adding them to a set (or UniqueVector).
Got ya, that should be much better for the 99.99% case where there are no 
non-runtime protocols. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

lanza updated this revision to Diff 295729.
lanza added a comment.

Update with John's suggestions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

Files:
  clang/include/clang/AST/DeclObjC.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/DeclObjC.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGObjCRuntime.h
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/test/CodeGenObjC/non-runtime-protocol.m
  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
@@ -118,6 +118,7 @@
 // CHECK-NEXT: ObjCExternallyRetained (SubjectMatchRule_variable_not_is_parameter, SubjectMatchRule_function, SubjectMatchRule_block, SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCMethodFamily (SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCNonLazyClass (SubjectMatchRule_objc_interface, SubjectMatchRule_objc_implementation)
+// CHECK-NEXT: ObjCNonRuntimeProtocol (SubjectMatchRule_objc_protocol)
 // CHECK-NEXT: ObjCPreciseLifetime (SubjectMatchRule_variable)
 // CHECK-NEXT: ObjCRequiresPropertyDefs (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: ObjCRequiresSuper (SubjectMatchRule_objc_method)
Index: clang/test/CodeGenObjC/non-runtime-protocol.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/non-runtime-protocol.m
@@ -0,0 +1,142 @@
+// RUN: not %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DPROTOEXPR -o - 2>&1 \
+// RUN: | FileCheck -check-prefix=PROTOEXPR %s
+
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DREDUNDANCY -o - \
+// RUN: | FileCheck -check-prefix=REDUNDANCY1 %s
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DREDUNDANCY -o - \
+// RUN: | FileCheck -check-prefix=REDUNDANCY2 %s
+
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DBASE -o - \
+// RUN: | FileCheck -check-prefix=NONFRAGILE %s
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=INHERITANCE %s
+
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-apple-darwin -fobjc-runtime=macosx-fragile-10.5 %s -DBASE -o - \
+// RUN: | FileCheck -check-prefix=FRAGILE %s
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-apple-darwin -fobjc-runtime=macosx-fragile-10.5 %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=FRAGILEINHERITANCE %s
+
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep %s -DBASE -o - \
+// RUN: | FileCheck -check-prefix=GNU %s
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=GNUINHERITANCE %s
+//
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep-2 %s -DBASE -o - \
+// RUN: | FileCheck -check-prefix=GNU2 %s
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep-2 %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=GNU2INHERITANCE %s
+
+__attribute__((objc_root_class))
+@interface Root
+@end
+@implementation Root
+@end
+
+#ifdef REDUNDANCY
+// REDUNDANCY1-NOT: _OBJC_CLASS_PROTOCOLS_$_Implementer{{.*}}_OBJC_PROTOCOL_$_B
+// REDUNDANCY2: _OBJC_CLASS_PROTOCOLS_$_Implementer{{.*}}_OBJC_PROTOCOL_$_C{{.*}}_OBJC_PROTOCOL_$_A
+@protocol C
+@end
+@protocol B 
+@end
+@protocol A 
+@end
+__attribute__((objc_non_runtime_protocol)) @protocol Alpha
+@end
+__attribute__((objc_non_runtime_protocol)) @protocol Beta
+@end
+@interface Implementer : Root 
+@end
+@implementation Implementer
+@end
+#endif
+
+#ifdef BASE
+// Confirm that we're not emitting protocol information for the
+// NONFRAGILE-NOT: OBJC_CLASS_NAME{{.*}}NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_$_PROTOCOL_INSTANCE_METHODS_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_$_PROTOCOL_CLASS_METHODS_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_PROTOCOL_$_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_LABEL_PROTOCOL_$_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_CLASS_PROTOCOLS_$_NonRuntimeImplementer
+// FRAGILE-NOT: OBJC_CLASS_NAME_.{{.*}}"Runtime\00"
+// FRAGILE-NOT: OBJC_PROTOCOL_NonRuntime
+// FRAGILE_NOT: OBJC_PROTOCOLS_NonRuntimeImplementer
+// GNU-NOT: private unnamed_addr constant {{.*}} c"NonRuntimeProtocol\00"
+// GNU-NOT: @.objc_protocol {{.*}}
+// GNU2-NOT: private unnamed_addr constant {{.*}} c"NonRuntimeProtocol\00"
+// GNU2-NOT: @.objc_protocol {{.*}}

[PATCH] D85528: [analyzer] Fix cast evaluation on scoped enums in ExprEngine

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Aha, ok, thanks, I now understand what the problem is because I was able to run 
the test before the patch and see how the patch changes the behavior.

What do you think about flattening the enum type out entirely? I.e., instead of 
`(unsigned char) conj_$2{enum ScopedSugared}` have `conj_$2{unsigned char}` 
from the start. Possibly same for unscoped enums. I don't think we actually 
extract any value from knowing that a symbol is an enum value. We also don't 
track enum types for concrete values (i.e., `nonloc::ConcreteInt` doesn't know 
whether it belongs to an enum).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85528

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

[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

> A value of an expression should have the same type and value-kind as the 
> expression.

F13121967: roflbot.jpg 

Unfortunately i'm pretty worried about our ability to actually achieve that in 
the near future; as of now we don't even oblige to a much simpler 
sanity-inducing contract that the value in the Environment //never changes// 
throughout its lifetime.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88477

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

[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

No, you got it all wrong again. I don't want to explain this one more time so 
let's talk about some basics: //A value of an expression should have the same 
type and value-kind as the expression//. Can we get there? How?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88477

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

[PATCH] D88278: [PowerPC] Add builtins for xvtdiv(dp|sp) and xvtsqrt(dp|sp).

[EsmeYi via Phabricator via cfe-commits]

Esme added inline comments.



Comment at: llvm/lib/Target/PowerPC/PPCInstrVSX.td:2584
 
+// Vector test software functions.
+def : Pat<(i32 (int_ppc_vsx_xvtdivdp v2f64:$A, v2f64:$B)),

steven.zhang wrote:
> Vector test for software divide and sqrt
I'll update it when committing. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88278

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

[PATCH] D88278: [PowerPC] Add builtins for xvtdiv(dp|sp) and xvtsqrt(dp|sp).

[EsmeYi via Phabricator via cfe-commits]

Esme added a comment.

In D88278#2306618 , @amyk wrote:

> Overall I think this LGTM.
>
> Please correct me if I am wrong but I think the description of the functions 
> need to be updated to:
>
>   int vec_test_swdiv(vector double v1, vector double v2);
>   int vec_test_swdivs(vector float v1, vector float v2);
>   int vec_test_swsqrt(vector double v1);
>   int vec_test_swsqrts(vector float v1);

Thanks! Updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88278

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

[PATCH] D88680: Add ability to turn off -fpch-instantiate-templates in clang-cl

[Shivanshu Goyal via Phabricator via cfe-commits]

shivanshu3 added a comment.

In D88680#2307564 , @rnk wrote:

> I think the flag was originally intended to be an internal -cc1 flag not 
> exposed to users. You should be able to work around your problem with 
> `-Xclang -fno-pch-instantiate-templates`, btw.
>
> 
>
> @zequanwu, can you patch this in locally and take over this patch? Please 
> address the hasFlag comment above, add a test to clang/test/Driver, and make 
> sure the flag works with `--driver-mode=gcc` and `--driver-mode=cl`. Follow 
> the examples of the other tests, run clang with `-###`, and make sure this 
> flag does or does not appear on the `-cc1` line.

Should we keep this flag internal though, or is it OK to make it a core flag? 
Our codebase had hundreds of compile errors caused by this default behavior 
which was hard to track down, so I'm guessing other people compiling with 
clang-cl might face similar problems? Or maybe we should re-think about making 
this the default behavior for clang-cl. Personally I don't have any strong 
preferences because we have a workaround now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88680

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

[PATCH] D88393: [cfe][M68K] (Patch 7/8) Basic Clang support

[Min-Yih Hsu via Phabricator via cfe-commits]

myhsu updated this revision to Diff 295706.
myhsu added a comment.

Update licenses


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

https://reviews.llvm.org/D88393

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/M680x0.cpp
  clang/lib/Basic/Targets/M680x0.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Sema/SemaDeclAttr.cpp

Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -5797,6 +5797,39 @@
   D->addAttr(::new (S.Context) MipsInterruptAttr(S.Context, AL, Kind));
 }
 
+static void handleM680x0InterruptAttr(Sema , Decl *D, const ParsedAttr ) {
+  if (!checkAttributeNumArgs(S, AL, 1))
+return;
+
+  if (!AL.isArgExpr(0)) {
+S.Diag(AL.getLoc(), diag::err_attribute_argument_type)
+<< AL << AANT_ArgumentIntegerConstant;
+return;
+  }
+
+  // FIXME: Check for decl - it should be void ()(void).
+
+  Expr *NumParamsExpr = static_cast(AL.getArgAsExpr(0));
+  auto MaybeNumParams = NumParamsExpr->getIntegerConstantExpr(S.Context);
+  if (!MaybeNumParams) {
+S.Diag(AL.getLoc(), diag::err_attribute_argument_type)
+<< AL << AANT_ArgumentIntegerConstant
+<< NumParamsExpr->getSourceRange();
+return;
+  }
+
+  unsigned Num = MaybeNumParams->getLimitedValue(255);
+  if ((Num & 1) || Num > 30) {
+S.Diag(AL.getLoc(), diag::err_attribute_argument_out_of_bounds)
+<< AL << (int)MaybeNumParams->getSExtValue()
+<< NumParamsExpr->getSourceRange();
+return;
+  }
+
+  D->addAttr(::new (S.Context) M680x0InterruptAttr(S.Context, AL, Num));
+  D->addAttr(UsedAttr::CreateImplicit(S.Context));
+}
+
 static void handleAnyX86InterruptAttr(Sema , Decl *D, const ParsedAttr ) {
   // Semantic checks for a function with the 'interrupt' attribute.
   // a) Must be a function.
@@ -6069,6 +6102,9 @@
   case llvm::Triple::mips:
 handleMipsInterruptAttr(S, D, AL);
 break;
+  case llvm::Triple::m680x0:
+handleM680x0InterruptAttr(S, D, AL);
+break;
   case llvm::Triple::x86:
   case llvm::Triple::x86_64:
 handleAnyX86InterruptAttr(S, D, AL);
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -8064,6 +8064,45 @@
   return false;
 }
 
+//===--===//
+// M680x0 ABI Implementation
+//===--===//
+
+namespace {
+
+class M680x0TargetCodeGenInfo : public TargetCodeGenInfo {
+public:
+  M680x0TargetCodeGenInfo(CodeGenTypes )
+  : TargetCodeGenInfo(std::make_unique(CGT)) {}
+  void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
+   CodeGen::CodeGenModule ) const override;
+};
+
+} // namespace
+
+// TODO Does not actually work right now
+void M680x0TargetCodeGenInfo::setTargetAttributes(
+const Decl *D, llvm::GlobalValue *GV, CodeGen::CodeGenModule ) const {
+  if (const FunctionDecl *FD = dyn_cast_or_null(D)) {
+if (const M680x0InterruptAttr *attr = FD->getAttr()) {
+  // Handle 'interrupt' attribute:
+  llvm::Function *F = cast(GV);
+
+  // Step 1: Set ISR calling convention.
+  F->setCallingConv(llvm::CallingConv::M680x0_INTR);
+
+  // Step 2: Add attributes goodness.
+  F->addFnAttr(llvm::Attribute::NoInline);
+
+  // ??? is this right
+  // Step 3: Emit ISR vector alias.
+  unsigned Num = attr->getNumber() / 2;
+  llvm::GlobalAlias::create(llvm::Function::ExternalLinkage,
+"__isr_" + Twine(Num), F);
+}
+  }
+}
+
 //===--===//
 // AVR ABI Implementation.
 //===--===//
Index: clang/lib/Basic/Targets/M680x0.h
===
--- /dev/null
+++ clang/lib/Basic/Targets/M680x0.h
@@ -0,0 +1,57 @@
+//===--- M680x0.h - Declare M680x0 target feature support ---*- C++ -*-===//
+//
+// 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
+//
+//===--===//
+//
+// This file declares M680x0 TargetInfo objects.
+//
+//===--===//
+
+#ifndef M680X0_H_LTNCIPAD
+#define M680X0_H_LTNCIPAD
+
+#include "OSTargets.h"
+#include "clang/Basic/TargetInfo.h"
+#include "clang/Basic/TargetOptions.h"
+#include "llvm/ADT/Triple.h"
+#include "llvm/Support/Compiler.h"
+
+namespace clang {
+namespace targets {
+
+class 

[PATCH] D72241: [clang-tidy] new altera single work item barrier check

[Frank Derry Wanye via Phabricator via cfe-commits]

ffrankies updated this revision to Diff 295698.
ffrankies removed a project: clang.
ffrankies added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

- Rebased code and fixed merge conflicts with  D66564 

- Added SingleWorkItemBarrierCheck.cpp to BUILD.gn


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

https://reviews.llvm.org/D72241

Files:
  clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
  clang-tools-extra/clang-tidy/altera/CMakeLists.txt
  clang-tools-extra/clang-tidy/altera/SingleWorkItemBarrierCheck.cpp
  clang-tools-extra/clang-tidy/altera/SingleWorkItemBarrierCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/altera-single-work-item-barrier.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/altera-single-work-item-barrier.cpp
  llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/altera/BUILD.gn

Index: llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/altera/BUILD.gn
===
--- llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/altera/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/altera/BUILD.gn
@@ -13,6 +13,7 @@
   ]
   sources = [
 "AlteraTidyModule.cpp",
+"SingleWorkItemBarrierCheck.cpp",
 "StructPackAlignCheck.cpp",
   ]
 }
Index: clang-tools-extra/test/clang-tidy/checkers/altera-single-work-item-barrier.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/altera-single-work-item-barrier.cpp
@@ -0,0 +1,294 @@
+// RUN: %check_clang_tidy -check-suffix=OLD %s altera-single-work-item-barrier %t -- -header-filter=.* "--" -cl-std=CL1.2 -c --include opencl-c.h -DOLD
+// RUN: %check_clang_tidy -check-suffix=NEW %s altera-single-work-item-barrier %t -- -header-filter=.* "--" -cl-std=CL2.0 -c --include opencl-c.h -DNEW
+// RUN: %check_clang_tidy -check-suffix=AOCOLD %s altera-single-work-item-barrier %t -- -config='{CheckOptions: [{key: altera-single-work-item-barrier.AOCVersion, value: 1701}]}' -header-filter=.* "--" -cl-std=CL1.2 -c --include opencl-c.h -DAOCOLD
+// RUN: %check_clang_tidy -check-suffix=AOCNEW %s altera-single-work-item-barrier %t -- -config='{CheckOptions: [{key: altera-single-work-item-barrier.AOCVersion, value: 1701}]}' -header-filter=.* "--" -cl-std=CL2.0 -c --include opencl-c.h -DAOCNEW
+
+#ifdef OLD
+void __kernel error_barrier_no_id(__global int * foo, int size) {
+  for (int j = 0; j < 256; j++) {
+	for (int i = 256; i < size; i+= 256) {
+  foo[j] += foo[j+i];
+}
+  }
+  barrier(CLK_GLOBAL_MEM_FENCE);
+  // CHECK-MESSAGES-OLD: :[[@LINE-7]]:15: warning: Kernel function 'error_barrier_no_id' does not call get_global_id or get_local_id and will be treated as single-work-item.{{[[:space:]]}}Barrier call at {{(\/)?([^\/\0]+(\/)?)+}}:[[@LINE-1]]:3 may error out [altera-single-work-item-barrier]
+  for (int i = 1; i < 256; i++) {
+	foo[0] += foo[i];
+  }
+}
+
+void __kernel success_barrier_global_id(__global int * foo, int size) {
+  barrier(CLK_GLOBAL_MEM_FENCE);
+  int tid = get_global_id(0);
+}
+
+void __kernel success_barrier_local_id(__global int * foo, int size) {
+  barrier(CLK_GLOBAL_MEM_FENCE);
+  int tid = get_local_id(0);
+}
+
+void __kernel success_barrier_both_ids(__global int * foo, int size) {
+  barrier(CLK_GLOBAL_MEM_FENCE);
+  int gid = get_global_id(0);
+  int lid = get_local_id(0);
+}
+
+void success_nokernel_barrier_no_id(__global int * foo, int size) {
+  for (int j = 0; j < 256; j++) {
+	for (int i = 256; i < size; i+= 256) {
+  foo[j] += foo[j+i];
+}
+  }
+  barrier(CLK_GLOBAL_MEM_FENCE);
+  for (int i = 1; i < 256; i++) {
+	foo[0] += foo[i];
+  }
+}
+
+void success_nokernel_barrier_global_id(__global int * foo, int size) {
+  barrier(CLK_GLOBAL_MEM_FENCE);
+  int tid = get_global_id(0);
+}
+
+void success_nokernel_barrier_local_id(__global int * foo, int size) {
+  barrier(CLK_GLOBAL_MEM_FENCE);
+  int tid = get_local_id(0);
+}
+
+void success_nokernel_barrier_both_ids(__global int * foo, int size) {
+  barrier(CLK_GLOBAL_MEM_FENCE);
+  int gid = get_global_id(0);
+  int lid = get_local_id(0);
+}
+#endif
+
+#ifdef NEW
+void __kernel error_barrier_no_id(__global int * foo, int size) {
+  for (int j = 0; j < 256; j++) {
+	for (int i = 256; i < size; i+= 256) {
+  foo[j] += foo[j+i];
+}
+  }
+  work_group_barrier(CLK_GLOBAL_MEM_FENCE);
+  // CHECK-MESSAGES-NEW: :[[@LINE-7]]:15: warning: Kernel function 'error_barrier_no_id' does not call get_global_id or get_local_id and will be treated as single-work-item.{{[[:space:]]}}Barrier call at {{(\/)?([^\/\0]+(\/)?)+}}:[[@LINE-1]]:3 may error out [altera-single-work-item-barrier]
+  for (int i = 1; i < 256; i++) {
+	foo[0] += foo[i];
+  }
+}
+
+void __kernel success_barrier_global_id(__global int * foo, int size) {
+  

[PATCH] D72218: [clang-tidy] new altera kernel name restriction check

[Frank Derry Wanye via Phabricator via cfe-commits]

ffrankies updated this revision to Diff 295697.
ffrankies removed a project: LLVM.
ffrankies added a comment.
Herald added a project: LLVM.

Addressed changes requested by @Eugene.Zelenko and @aaron.ballman


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

https://reviews.llvm.org/D72218

Files:
  clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
  clang-tools-extra/clang-tidy/altera/CMakeLists.txt
  clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp
  clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/altera-kernel-name-restriction.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/KERNEL.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/VHDL.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/Verilog.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/kernel.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/kernel.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/other_Verilog.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/otherdir/vhdl.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/otherthing.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/dir/kernel.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/kernel.cl/foo.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/verilog.cl/foo.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/vhdl.cl/foo.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some_kernel.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/somedir/verilog.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/thing.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vERILOG.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/verilog.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl.CL
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl_number_two.cl
  clang-tools-extra/test/clang-tidy/checkers/altera-kernel-name-restriction.cpp
  llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/altera/BUILD.gn

Index: llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/altera/BUILD.gn
===
--- llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/altera/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/altera/BUILD.gn
@@ -13,6 +13,7 @@
   ]
   sources = [
 "AlteraTidyModule.cpp",
+"KernelNameRestrictionCheck.cpp",
 "StructPackAlignCheck.cpp",
   ]
 }
Index: clang-tools-extra/test/clang-tidy/checkers/altera-kernel-name-restriction.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/altera-kernel-name-restriction.cpp
@@ -0,0 +1,50 @@
+// RUN: %check_clang_tidy %s altera-kernel-name-restriction %t -- -- -I%S/Inputs/altera-kernel-name-restriction
+
+// These are the banned kernel filenames, and should trigger warnings
+#include "kernel.cl"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: including 'kernel.cl' may cause additional compilation errors due to the name of the kernel source file; consider renaming the included kernel source file [altera-kernel-name-restriction]
+#include "Verilog.cl"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: including 'Verilog.cl' may cause additional compilation errors due to the name of the kernel source file; consider renaming the included kernel source file [altera-kernel-name-restriction]
+#include "VHDL.cl"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: including 'VHDL.cl' may cause additional compilation errors due to the name of the kernel source file; consider renaming the included kernel source file [altera-kernel-name-restriction]
+
+// The warning should be triggered regardless of capitalization
+#include "KERNEL.cl"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: including 'KERNEL.cl' may cause additional compilation errors due to the name of the kernel source file; consider renaming the included kernel source file [altera-kernel-name-restriction]
+#include "vERILOG.cl"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: including 'vERILOG.cl' may cause additional compilation errors due to the name of the kernel source file; consider 

[PATCH] D88680: Add ability to turn off -fpch-instantiate-templates in clang-cl

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a subscriber: zequanwu.
rnk added a comment.

I think the flag was originally intended to be an internal -cc1 flag not 
exposed to users. You should be able to work around your problem with `-Xclang 
-fno-pch-instantiate-templates`, btw.



@zequanwu, can you patch this in locally and take over this patch? Please 
address the hasFlag comment above, add a test to clang/test/Driver, and make 
sure the flag works with `--driver-mode=gcc` and `--driver-mode=cl`. Follow the 
examples of the other tests, run clang with `-###`, and make sure this flag 
does or does not appear on the `-cc1` line.




Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1217
+  // precomp using /Yc
+  if (!Args.hasArg(options::OPT_fno_pch_instantiate_templates))
+CmdArgs.push_back(Args.MakeArgString("-fpch-instantiate-templates"));

This should probably be `hasFlag(fpch_ins..., fno_pch_inst..., true)` so that 
`-fno-pch-inst -fpch-inst...` works.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88680

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

[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

So was the bug we were saying there were falsely equivalent or falsely not 
equivalent?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88665

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

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 295696.
MaskRay added a comment.

Improve tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88712

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/asm-label.c
  clang/test/CodeGen/redefine_extname.c


Index: clang/test/CodeGen/redefine_extname.c
===
--- clang/test/CodeGen/redefine_extname.c
+++ clang/test/CodeGen/redefine_extname.c
@@ -30,3 +30,9 @@
 int baz() { return foo_static(); }
 // CHECK-NOT: call i32 @bar_static()
 
+// Check that pragma redefine_extname applies to builtin functions.
+typedef unsigned long size_t;
+extern void *memcpy(void *, const void *, size_t);
+#pragma redefine_extname memcpy __GI_memcpy
+void *test_memcpy(void *dst, const void *src, size_t n) { return memcpy(dst, 
src, n); }
+// CHECK: call i8* @__GI_memcpy(
Index: clang/test/CodeGen/asm-label.c
===
--- clang/test/CodeGen/asm-label.c
+++ clang/test/CodeGen/asm-label.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple=i686-pc-linux-gnu -emit-llvm %s -o - | FileCheck %s 
--check-prefix=LINUX
-// RUN: %clang_cc1 -triple=i686-apple-darwin9 -emit-llvm %s -o - | FileCheck 
%s --check-prefix=DARWIN
+// RUN: %clang_cc1 -triple=i686-pc-linux-gnu -emit-llvm %s -o - | FileCheck %s 
--check-prefixes=CHECK,LINUX
+// RUN: %clang_cc1 -triple=i686-apple-darwin9 -emit-llvm %s -o - | FileCheck 
%s --check-prefixes=CHECK,DARWIN
 
 char *strerror(int) asm("alias");
 int x __asm("foo");
@@ -17,3 +17,28 @@
 // DARWIN: @"\01bar" = internal global i32 0
 // DARWIN: @"\01foo" = global i32 0
 // DARWIN: declare i8* @"\01alias"(i32)
+
+extern void *memcpy(void *__restrict, const void *__restrict, unsigned long);
+extern __typeof(memcpy) memcpy asm("__GI_memcpy");
+void test_memcpy(void *dst, void *src, unsigned long n) {
+  memcpy(dst, src, n);
+}
+
+// CHECK-LABEL: @test_memcpy(
+// LINUX: call i8* @__GI_memcpy(
+// LINUX:   declare i8* @__GI_memcpy(i8*, i8*, i32)
+
+// DARWIN:call i8* @"\01__GI_memcpy"(
+// DARWIN:  declare i8* @"\01__GI_memcpy"(i8*, i8*, i32)
+
+long lrint(double x) asm("__GI_lrint");
+long test_lrint(double x) {
+  return lrint(x);
+}
+
+// CHECK-LABEL: @test_lrint(
+// LINUX: call i32 @__GI_lrint(
+// LINUX:   declare i32 @__GI_lrint(double)
+
+// DARWIN:call i32 @"\01__GI_lrint"(
+// DARWIN:  declare i32 @"\01__GI_lrint"(double)
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -1663,13 +1663,18 @@
Result.Val.getFloat()));
   }
 
+  // If the builtin has been declared explicitly with an assembler label,
+  // disable the specialized emitting below.
+  const unsigned BuiltinIDIfNoAsmLabel =
+  FD->hasAttr() ? 0 : BuiltinID;
+
   // There are LLVM math intrinsics/instructions corresponding to math library
   // functions except the LLVM op will never set errno while the math library
   // might. Also, math builtins have the same semantics as their math library
   // twins. Thus, we can transform math library and builtin calls to their
   // LLVM counterparts if the call is marked 'const' (known to never set 
errno).
   if (FD->hasAttr()) {
-switch (BuiltinID) {
+switch (BuiltinIDIfNoAsmLabel) {
 case Builtin::BIceil:
 case Builtin::BIceilf:
 case Builtin::BIceill:
@@ -1946,7 +1951,7 @@
 }
   }
 
-  switch (BuiltinID) {
+  switch (BuiltinIDIfNoAsmLabel) {
   default: break;
   case Builtin::BI__builtin___CFStringMakeConstantString:
   case Builtin::BI__builtin___NSStringMakeConstantString:


Index: clang/test/CodeGen/redefine_extname.c
===
--- clang/test/CodeGen/redefine_extname.c
+++ clang/test/CodeGen/redefine_extname.c
@@ -30,3 +30,9 @@
 int baz() { return foo_static(); }
 // CHECK-NOT: call i32 @bar_static()
 
+// Check that pragma redefine_extname applies to builtin functions.
+typedef unsigned long size_t;
+extern void *memcpy(void *, const void *, size_t);
+#pragma redefine_extname memcpy __GI_memcpy
+void *test_memcpy(void *dst, const void *src, size_t n) { return memcpy(dst, src, n); }
+// CHECK: call i8* @__GI_memcpy(
Index: clang/test/CodeGen/asm-label.c
===
--- clang/test/CodeGen/asm-label.c
+++ clang/test/CodeGen/asm-label.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple=i686-pc-linux-gnu -emit-llvm %s -o - | FileCheck %s --check-prefix=LINUX
-// RUN: %clang_cc1 -triple=i686-apple-darwin9 -emit-llvm %s -o - | FileCheck %s --check-prefix=DARWIN
+// RUN: %clang_cc1 -triple=i686-pc-linux-gnu -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,LINUX
+// RUN: %clang_cc1 

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

[Fangrui Song via Phabricator via cfe-commits]

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

rL131311  added `asm()` support for builtin 
functions, but `asm()` for builtins with
specialized emitting (e.g. memcpy, various math functions) still do not work.

This patch makes these functions work for `asm()` and `#pragma 
redefine_extname`.
glibc uses `asm()` to redirect internal libc function calls to hidden aliases.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88712

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/asm-label.c
  clang/test/CodeGen/redefine_extname.c


Index: clang/test/CodeGen/redefine_extname.c
===
--- clang/test/CodeGen/redefine_extname.c
+++ clang/test/CodeGen/redefine_extname.c
@@ -30,3 +30,9 @@
 int baz() { return foo_static(); }
 // CHECK-NOT: call i32 @bar_static()
 
+// Check that pragma redefine_extname applies to builtin functions.
+typedef unsigned long size_t;
+extern void *memcpy(void *, const void *, size_t);
+#pragma redefine_extname memcpy __GI_memcpy
+void *test_memcpy(void *dst, const void *src, size_t n) { return memcpy(dst, 
src, n); }
+// CHECK: call i8* @__GI_memcpy(
Index: clang/test/CodeGen/asm-label.c
===
--- clang/test/CodeGen/asm-label.c
+++ clang/test/CodeGen/asm-label.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple=i686-pc-linux-gnu -emit-llvm %s -o - | FileCheck %s 
--check-prefix=LINUX
-// RUN: %clang_cc1 -triple=i686-apple-darwin9 -emit-llvm %s -o - | FileCheck 
%s --check-prefix=DARWIN
+// RUN: %clang_cc1 -triple=i686-pc-linux-gnu -emit-llvm %s -o - | FileCheck %s 
--check-prefixes=CHECK,LINUX
+// RUN: %clang_cc1 -triple=i686-apple-darwin9 -emit-llvm %s -o - | FileCheck 
%s --check-prefixes=CHECK,DARWIN
 
 char *strerror(int) asm("alias");
 int x __asm("foo");
@@ -17,3 +17,22 @@
 // DARWIN: @"\01bar" = internal global i32 0
 // DARWIN: @"\01foo" = global i32 0
 // DARWIN: declare i8* @"\01alias"(i32)
+
+typedef unsigned long size_t;
+extern void *memcpy (void *__restrict, const void *__restrict, size_t);
+extern __typeof(memcpy) memcpy asm("__GI_memcpy");
+void *test_memcpy(void *dst, void *src, size_t n) {
+  memcpy(dst, src, n);
+}
+
+// CHECK-LABEL: @test_lrint(
+// LINUX: call i32 @__GI_lrint(
+// LINUX:   declare i32 @__GI_lrint(double)
+
+// DARWIN:call i32 @"\01__GI_lrint"(
+// DARWIN:  declare i32 @"\01__GI_lrint"(double)
+
+long lrint(double x) asm("__GI_lrint");
+long test_lrint(double x) {
+  return lrint(x);
+}
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -1663,13 +1663,18 @@
Result.Val.getFloat()));
   }
 
+  // If the builtin has been declared explicitly with an assembler label,
+  // disable the specialized emitting below.
+  const unsigned BuiltinIDIfNoAsmLabel =
+  FD->hasAttr() ? 0 : BuiltinID;
+
   // There are LLVM math intrinsics/instructions corresponding to math library
   // functions except the LLVM op will never set errno while the math library
   // might. Also, math builtins have the same semantics as their math library
   // twins. Thus, we can transform math library and builtin calls to their
   // LLVM counterparts if the call is marked 'const' (known to never set 
errno).
   if (FD->hasAttr()) {
-switch (BuiltinID) {
+switch (BuiltinIDIfNoAsmLabel) {
 case Builtin::BIceil:
 case Builtin::BIceilf:
 case Builtin::BIceill:
@@ -1946,7 +1951,7 @@
 }
   }
 
-  switch (BuiltinID) {
+  switch (BuiltinIDIfNoAsmLabel) {
   default: break;
   case Builtin::BI__builtin___CFStringMakeConstantString:
   case Builtin::BI__builtin___NSStringMakeConstantString:


Index: clang/test/CodeGen/redefine_extname.c
===
--- clang/test/CodeGen/redefine_extname.c
+++ clang/test/CodeGen/redefine_extname.c
@@ -30,3 +30,9 @@
 int baz() { return foo_static(); }
 // CHECK-NOT: call i32 @bar_static()
 
+// Check that pragma redefine_extname applies to builtin functions.
+typedef unsigned long size_t;
+extern void *memcpy(void *, const void *, size_t);
+#pragma redefine_extname memcpy __GI_memcpy
+void *test_memcpy(void *dst, const void *src, size_t n) { return memcpy(dst, src, n); }
+// CHECK: call i8* @__GI_memcpy(
Index: clang/test/CodeGen/asm-label.c
===
--- clang/test/CodeGen/asm-label.c
+++ clang/test/CodeGen/asm-label.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple=i686-pc-linux-gnu -emit-llvm %s -o - | FileCheck %s --check-prefix=LINUX
-// RUN: %clang_cc1 -triple=i686-apple-darwin9 -emit-llvm %s -o - | 

[PATCH] D74144: [OPENMP50]Add basic support for array-shaping operation.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.



In D74144#2307454 , @cchen wrote:

> @ABataev, the below test is extracted from Sollve test suite and Clang now 
> emit:
>
>   test.c:17:35: error: subscripted value is not an array or pointer
>   #pragma omp target update to( (([N][N])foo)[1:M] )
> ^
>   test.c:17:5: error: expected at least one 'to' clause or 'from' clause 
> specified to '#pragma omp target update'
>   #pragma omp target update to( (([N][N])foo)[1:M] )
>
> This error message came from the `ActOnOMPArraySectionExpr` which is called 
> inside `ParsePostfixExpressionSuffix`. The issue is that the base expression 
> in `ActOnOMPArraySectionExpr` looks like:
>
>   ParenExpr 0x122859be0 '' lvalue
>   `-OMPArrayShapingExpr 0x122859b98 '' lvalue
> |-IntegerLiteral 0x122859b38 'int' 5
> |-IntegerLiteral 0x122859b58 'int' 5
> `-DeclRefExpr 0x122859b78 'int *' lvalue Var 0x1228599d0 'foo' 'int *'
>
> which is not a base that we would expect in an array section expr. I've tried 
> relaxing the base type check in `ActOnOMPArraySectionExpr` but not sure it's 
> the way to go. (or should I just extract the DeclReExpr from ArrayShapingExpr 
> before calling `ActOnOMPArraySectionExpr`?)
>
>   #define N 5
>   #define M 3
>   
>   int main(void) {
>   int tmp[N][N];
>   for(int i=0; i   for(int j=0; j   tmp[i][j] = N*i + j;
>   
>   int *foo = [0][0];
>   
>   // This compiles just fine
>   //#pragma omp target update to( ([N][N])foo )
>   
>   // This is rejected by the compiler
>   #pragma omp target update to( (([N][N])foo)[1:M] )
>   }

I don't think it is allowed by the standard.

According to the standard, The shape-operator can appear only in clauses where 
it is explicitly allowed.
In this case, array shaping is used as a base expression of array section (or 
subscript) expression, which does not meet the standard. Tje array sjaping 
operation is not used in clause, instead it is used as a base subexpression of 
another expression.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74144

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

[PATCH] D74144: [OPENMP50]Add basic support for array-shaping operation.

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.
Herald added subscribers: sstefan1, yaxunl.

@ABataev, the below test is extracted from Sollve test suite and Clang now emit:

  test.c:17:35: error: subscripted value is not an array or pointer
  #pragma omp target update to( (([N][N])foo)[1:M] )
^
  test.c:17:5: error: expected at least one 'to' clause or 'from' clause 
specified to '#pragma omp target update'
  #pragma omp target update to( (([N][N])foo)[1:M] )

This error message came from the `ActOnOMPArraySectionExpr` which is called 
inside `ParsePostfixExpressionSuffix`. The issue is that the base expression in 
`ActOnOMPArraySectionExpr` looks like:

  ParenExpr 0x122859be0 '' lvalue
  `-OMPArrayShapingExpr 0x122859b98 '' lvalue
|-IntegerLiteral 0x122859b38 'int' 5
|-IntegerLiteral 0x122859b58 'int' 5
`-DeclRefExpr 0x122859b78 'int *' lvalue Var 0x1228599d0 'foo' 'int *'

which is not a base that we would expect in an array section expr. I've tried 
relaxing the base type check in `ActOnOMPArraySectionExpr` but not sure it's 
the way to go. (or should I just extract the DeclReExpr from ArrayShapingExpr 
before calling `ActOnOMPArraySectionExpr`?)

  #define N 5
  #define M 3
  
  int main(void) {
  int tmp[N][N];
  for(int i=0; ihttps://reviews.llvm.org/D74144/new/

https://reviews.llvm.org/D74144

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

[PATCH] D88338: [clangd] clangd --check: standalone diagnosis of common problems

[Hongtao Yu via Phabricator via cfe-commits]

hoy added inline comments.



Comment at: clang-tools-extra/clangd/tool/Check.cpp:243
+  size_t N = 50;
+  auto xxx = std::string(N, 'x');
+)cpp";

Hi, I'm seeing an error with this "tweak: ExpandAutoType ==> FAIL: Could not 
deduce type for 'auto' type". It's probably due to that the std string header 
was not available when running `check.test`, thus the test failed. Is it 
expected that the header is available when test is run? Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88338

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

[PATCH] D88338: [clangd] clangd --check: standalone diagnosis of common problems

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

rsmith added inline comments.



Comment at: clang-tools-extra/clangd/test/check.test:1
+# RUN: clangd -log=verbose -check 2>&1 | FileCheck -strict-whitespace %s
+

This test implicitly parses a source file that `#include`s standard library 
headers, and fails if those headers aren't available; this causes the test to 
fail in some build environments. Would it be possible to make this test work in 
freestanding environments?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88338

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

[PATCH] D88345: [CUDA] Allow local `static const {__constant__, __device__}` variables.

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

I've verified that clang with this patch can compile Tensorflow and that it can 
also compile `cooperative_groups.h` from CUDA-11.




Comment at: clang/test/SemaCUDA/device-var-init.cu:404
 __host__ __device__ void hd_sema() {
   static int x = 42;
 }

yaxunl wrote:
> how does this work in device compilation? Is this equivalent to `static 
> __device__ int x = 42`?
Correct. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88345

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

[libclc] 1c1a810 - libclc: Use find_package to find Python 3 and require it

[Aaron Puchert via cfe-commits]

Author: Aaron Puchert
Date: 2020-10-01T22:31:33+02:00
New Revision: 1c1a8105580784c96212db1afc097a844740bc69

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

LOG: libclc: Use find_package to find Python 3 and require it

The script's shebang wants Python 3, so we use FindPython3. The
original code didn't work when an unversioned python was not available.
This is explicitly allowed in PEP 394. ("Distributors may choose to set
the behavior of the python command as follows: python2, python3, not
provide python command, allow python to be configurable by an end user
or a system administrator.")

Also I think it's actually required, so let the configuration fail if we
can't find it.

Lastly remove the shebang, since the script is only run via interpreter
and doesn't have the executable bit set anyway.

Reviewed By: jvesely

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

Added: 


Modified: 
libclc/CMakeLists.txt
libclc/generic/lib/gen_convert.py

Removed: 




diff  --git a/libclc/CMakeLists.txt b/libclc/CMakeLists.txt
index 1a77a378e192..b8b5ceff086c 100644
--- a/libclc/CMakeLists.txt
+++ b/libclc/CMakeLists.txt
@@ -184,11 +184,11 @@ if( ENABLE_RUNTIME_SUBNORMAL )
DESTINATION ${CMAKE_INSTALL_DATADIR}/clc )
 endif()
 
-find_program( PYTHON python )
+find_package( Python3 REQUIRED COMPONENTS Interpreter )
 file( TO_CMAKE_PATH ${CMAKE_SOURCE_DIR}/generic/lib/gen_convert.py script_loc )
 add_custom_command(
OUTPUT convert.cl
-   COMMAND ${PYTHON} ${script_loc} > convert.cl
+   COMMAND ${Python3_EXECUTABLE} ${script_loc} > convert.cl
DEPENDS ${script_loc} )
 add_custom_target( "generate_convert.cl" DEPENDS convert.cl )
 

diff  --git a/libclc/generic/lib/gen_convert.py 
b/libclc/generic/lib/gen_convert.py
index 5c87fcbe1aba..7e649faa7dfc 100644
--- a/libclc/generic/lib/gen_convert.py
+++ b/libclc/generic/lib/gen_convert.py
@@ -1,5 +1,3 @@
-#!/usr/bin/env python3
-
 # OpenCL built-in library: type conversion functions
 #
 # Copyright (c) 2013 Victor Oliveira 



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

[PATCH] D88524: [CUDA][HIP] Fix bound arch for offload action for fat binary

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/lib/Basic/Cuda.cpp:95
 const char *CudaArchToString(CudaArch A) {
+  if (A == CudaArch::UNUSED)
+return "";

tra wrote:
> You could add a `{CudaArch::UNUSED, "", ""}` to the table above instead of 
> explicit checks here.
will do when committing


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

https://reviews.llvm.org/D88524

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

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Wouter van Oortmerssen via Phabricator via cfe-commits]

aardappel added inline comments.



Comment at: llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll:47
+;
+; SINGLE-4: .debug_types contents:
+; SINGLE-4: 0x: Type Unit: {{.*}} version = 0x0004, abbr_offset

I guess this doesn't actually test that only one copy of these remain after 
linking? That's already tested in LLD?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D88526: [clang][Sema] Fix PR47676: Handle dependent AltiVec C-style cast

[Hubert Tong via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG35ecc7fe49ba: [clang][Sema] Fix PR47676: Handle dependent 
AltiVec C-style cast (authored by hubert.reinterpretcast).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88526

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaTemplate/pr47676.cpp


Index: clang/test/SemaTemplate/pr47676.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/pr47676.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple=powerpc64le-unknown-linux-gnu \
+// RUN:-target-feature +altivec -fsyntax-only -ast-dump \
+// RUN:-xc++ < %s 2>&1 \
+// RUN:   | FileCheck %s
+
+// Ensures that casts to AltiVec type with a dependent expression operand does
+// not hit the assertion failure reported in PR47676. Further checks that casts
+// to AltiVec type with a dependent expression operand is, on instantiation,
+// able to correctly differentiate between a splat case and a bitcast case.
+template  void f(T *tp) {
+  extern void g(int, ...);
+  g(0, (__vector int)(*tp));
+  g(0, (__vector int)*tp);
+}
+
+void g(void) {
+  f<__vector float>(nullptr);
+//  CHECK: | |-FunctionDecl {{.*}} f 'void (__vector float *)'
+
+//  CHECK: |   | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   |   `-ImplicitCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   | `-ImplicitCastExpr {{.*}}'__vector float' 

+
+//  CHECK: | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   `-ImplicitCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | `-ImplicitCastExpr {{.*}}'__vector float' 

+
+  f(nullptr);
+//  CHECK: | `-FunctionDecl {{.*}} f 'void (double *)'
+
+//  CHECK: | | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | |   `-ImplicitCastExpr {{.*}} 'int' 
+// CHECK-NEXT: | | `-ImplicitCastExpr {{.*}}'double' 
+
+//  CHECK: |   `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | `-ImplicitCastExpr {{.*}} 'int' 
+// CHECK-NEXT: |   `-ImplicitCastExpr {{.*}}:'double' 
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -7408,7 +7408,7 @@
 }
 if (PE || PLE->getNumExprs() == 1) {
   Expr *E = (PE ? PE->getSubExpr() : PLE->getExpr(0));
-  if (!E->getType()->isVectorType())
+  if (!E->isTypeDependent() && !E->getType()->isVectorType())
 isVectorLiteral = true;
 }
 else


Index: clang/test/SemaTemplate/pr47676.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/pr47676.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple=powerpc64le-unknown-linux-gnu \
+// RUN:-target-feature +altivec -fsyntax-only -ast-dump \
+// RUN:-xc++ < %s 2>&1 \
+// RUN:   | FileCheck %s
+
+// Ensures that casts to AltiVec type with a dependent expression operand does
+// not hit the assertion failure reported in PR47676. Further checks that casts
+// to AltiVec type with a dependent expression operand is, on instantiation,
+// able to correctly differentiate between a splat case and a bitcast case.
+template  void f(T *tp) {
+  extern void g(int, ...);
+  g(0, (__vector int)(*tp));
+  g(0, (__vector int)*tp);
+}
+
+void g(void) {
+  f<__vector float>(nullptr);
+//  CHECK: | |-FunctionDecl {{.*}} f 'void (__vector float *)'
+
+//  CHECK: |   | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   |   `-ImplicitCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   | `-ImplicitCastExpr {{.*}}'__vector float' 
+
+//  CHECK: | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   `-ImplicitCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | `-ImplicitCastExpr {{.*}}'__vector float' 
+
+  f(nullptr);
+//  CHECK: | `-FunctionDecl {{.*}} f 'void (double *)'
+
+//  CHECK: | | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | |   `-ImplicitCastExpr {{.*}} 'int' 
+// CHECK-NEXT: | | `-ImplicitCastExpr {{.*}}'double' 
+
+//  CHECK: |   `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | `-ImplicitCastExpr {{.*}} 'int' 
+// CHECK-NEXT: |   `-ImplicitCastExpr {{.*}}:'double' 
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -7408,7 +7408,7 @@
 }
 if (PE || PLE->getNumExprs() == 1) {
   Expr *E = (PE ? PE->getSubExpr() : PLE->getExpr(0));
-  if (!E->getType()->isVectorType())
+  if (!E->isTypeDependent() && !E->getType()->isVectorType())
 isVectorLiteral = true;
 }
 else
___
cfe-commits mailing list

[clang] 35ecc7f - [clang][Sema] Fix PR47676: Handle dependent AltiVec C-style cast

[Hubert Tong via cfe-commits]

Author: Hubert Tong
Date: 2020-10-01T15:57:01-04:00
New Revision: 35ecc7fe49ba881a77e8146b51870a60a52b211f

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

LOG: [clang][Sema] Fix PR47676: Handle dependent AltiVec C-style cast

Fix premature decision in the presence of type-dependent expression
operands on whether AltiVec vector initializations from single
expressions are "splat" operations.

Verify that the instantiation is able to determine the correct cast
semantics for both the scalar type and the vector type case.

Note that, because the change only affects the single-expression
case (and the target type is an AltiVec-style vector type), the
replacement of a parenthesized list with a parenthesized expression
does not change the semantics of the program in a program-observable
manner.

Reviewed By: aaron.ballman

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

Added: 
clang/test/SemaTemplate/pr47676.cpp

Modified: 
clang/lib/Sema/SemaExpr.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 22840dd3dfe3..e51b27626184 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -7408,7 +7408,7 @@ Sema::ActOnCastExpr(Scope *S, SourceLocation LParenLoc,
 }
 if (PE || PLE->getNumExprs() == 1) {
   Expr *E = (PE ? PE->getSubExpr() : PLE->getExpr(0));
-  if (!E->getType()->isVectorType())
+  if (!E->isTypeDependent() && !E->getType()->isVectorType())
 isVectorLiteral = true;
 }
 else

diff  --git a/clang/test/SemaTemplate/pr47676.cpp 
b/clang/test/SemaTemplate/pr47676.cpp
new file mode 100644
index ..428607097c96
--- /dev/null
+++ b/clang/test/SemaTemplate/pr47676.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple=powerpc64le-unknown-linux-gnu \
+// RUN:-target-feature +altivec -fsyntax-only -ast-dump \
+// RUN:-xc++ < %s 2>&1 \
+// RUN:   | FileCheck %s
+
+// Ensures that casts to AltiVec type with a dependent expression operand does
+// not hit the assertion failure reported in PR47676. Further checks that casts
+// to AltiVec type with a dependent expression operand is, on instantiation,
+// able to correctly 
diff erentiate between a splat case and a bitcast case.
+template  void f(T *tp) {
+  extern void g(int, ...);
+  g(0, (__vector int)(*tp));
+  g(0, (__vector int)*tp);
+}
+
+void g(void) {
+  f<__vector float>(nullptr);
+//  CHECK: | |-FunctionDecl {{.*}} f 'void (__vector float *)'
+
+//  CHECK: |   | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   |   `-ImplicitCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   | `-ImplicitCastExpr {{.*}}'__vector float' 

+
+//  CHECK: | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   `-ImplicitCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | `-ImplicitCastExpr {{.*}}'__vector float' 

+
+  f(nullptr);
+//  CHECK: | `-FunctionDecl {{.*}} f 'void (double *)'
+
+//  CHECK: | | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | |   `-ImplicitCastExpr {{.*}} 'int' 
+// CHECK-NEXT: | | `-ImplicitCastExpr {{.*}}'double' 
+
+//  CHECK: |   `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | `-ImplicitCastExpr {{.*}} 'int' 
+// CHECK-NEXT: |   `-ImplicitCastExpr {{.*}}:'double' 
+}



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

[PATCH] D88345: [CUDA] Allow local `static const {__constant__, __device__}` variables.

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/test/SemaCUDA/device-var-init.cu:404
 __host__ __device__ void hd_sema() {
   static int x = 42;
 }

how does this work in device compilation? Is this equivalent to `static 
__device__ int x = 42`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88345

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

[clang] de47e71 - [CMake][Fuchsia] Don't set WIN32 API, rely on autodetection

[Petr Hosek via cfe-commits]

Author: Petr Hosek
Date: 2020-10-01T12:35:52-07:00
New Revision: de47e7122f69d56399c4f8864ba279e5ce635970

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

LOG: [CMake][Fuchsia] Don't set WIN32 API, rely on autodetection

We prefer autodetection here to avoid persisting this configuration
in the generated __config header which is shared across targets.

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

Added: 


Modified: 
clang/cmake/caches/Fuchsia-stage2.cmake

Removed: 




diff  --git a/clang/cmake/caches/Fuchsia-stage2.cmake 
b/clang/cmake/caches/Fuchsia-stage2.cmake
index e00b64073ca5..98db622ba34b 100644
--- a/clang/cmake/caches/Fuchsia-stage2.cmake
+++ b/clang/cmake/caches/Fuchsia-stage2.cmake
@@ -84,7 +84,6 @@ if(WIN32)
   set(RUNTIMES_${target}_CMAKE_SYSTEM_NAME Windows CACHE STRING "")
   set(RUNTIMES_${target}_CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "")
   set(RUNTIMES_${target}_LIBCXX_ABI_VERSION 2 CACHE STRING "")
-  set(RUNTIMES_${target}_LIBCXX_HAS_WIN32_THREAD_API ON CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_FILESYSTEM OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_ABI_LINKER_SCRIPT OFF CACHE BOOL "")



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

[PATCH] D88694: [CMake][Fuchsia] Don't set WIN32 API, rely on autodetection

[Petr Hosek 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 rGde47e7122f69: [CMake][Fuchsia] Dont set WIN32 API, 
rely on autodetection (authored by phosek).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88694

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -84,7 +84,6 @@
   set(RUNTIMES_${target}_CMAKE_SYSTEM_NAME Windows CACHE STRING "")
   set(RUNTIMES_${target}_CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "")
   set(RUNTIMES_${target}_LIBCXX_ABI_VERSION 2 CACHE STRING "")
-  set(RUNTIMES_${target}_LIBCXX_HAS_WIN32_THREAD_API ON CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_FILESYSTEM OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_ABI_LINKER_SCRIPT OFF CACHE BOOL "")


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -84,7 +84,6 @@
   set(RUNTIMES_${target}_CMAKE_SYSTEM_NAME Windows CACHE STRING "")
   set(RUNTIMES_${target}_CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "")
   set(RUNTIMES_${target}_LIBCXX_ABI_VERSION 2 CACHE STRING "")
-  set(RUNTIMES_${target}_LIBCXX_HAS_WIN32_THREAD_API ON CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_FILESYSTEM OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_ABI_LINKER_SCRIPT OFF CACHE BOOL "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87989: [Flang][Driver] Add infrastructure for basic frontend actions and file I/O

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

@CarolineConcatto thank you again for working on this! The structure is good, 
but IMHO this patch could be polished a bit more. Overall:

- could you make sure that this patch does not change the output from `clang 
-help`?
- doxygen comments are consistent
- unittests are a bit better documentated (what and why is tested?)
- use `llvm/Support/FileSystem.h`  instead of ``

More comments inline. Otherwise, I think that this is almost ready :)




Comment at: clang/include/clang/Driver/Options.td:2795
 def object : Flag<["-"], "object">;
-def o : JoinedOrSeparate<["-"], "o">, Flags<[DriverOption, RenderAsInput, 
CC1Option, CC1AsOption]>,
+def o : JoinedOrSeparate<["-"], "o">, Flags<[DriverOption, RenderAsInput, 
CC1Option, CC1AsOption, FC1Option]>,
   HelpText<"Write output to ">, MetaVarName<"">;

What about FlangOption?



Comment at: clang/lib/Driver/Driver.cpp:1576-1579
+  } else {
+ExcludedFlagsBitmask |= options::FlangOption;
+ExcludedFlagsBitmask |= options::FC1Option;
+  }

With this, the following is empty:
```
$ bin/clang --help | grep help
```

This is what I'd expect instead (i.e. the behavior shouldn't change):
```
$ bin/clang --help | grep help
  --help-hidden   Display help for hidden options
  -help   Display available options
```

This needs a bit more polishing.



Comment at: clang/test/Driver/immediate-options.c:5
 // HELP-NOT: driver-mode
+// HEKP-NOT: test-io
 

HELP instead of HEKP

Could add a comment why this particular test is here?



Comment at: flang/include/flang/Frontend/CompilerInstance.h:58
+
+  /// }
+  /// @name Compiler Invocation

I can't see a matching `/// }` for this. DELETEME



Comment at: flang/include/flang/Frontend/CompilerInstance.h:85
+  /// CompilerInvocation object.
+  /// Note that this routine may write output to 'stderr'.
+  /// \param act - The action to execute.

From what I can see, this is not the case. Could you update?



Comment at: flang/include/flang/Frontend/FrontendAction.h:23
+protected:
+  /// @}
+  /// @name Implementation Action Interface

DELETEME (missing matching `/// {`)



Comment at: flang/include/flang/Frontend/FrontendAction.h:42
+
+  /// @}
+  /// @name Compiler Instance Access

I think that this should be on line 38.



Comment at: flang/include/flang/Frontend/FrontendOptions.h:16
 #include 
 namespace Fortran::frontend {
 

[nit] Empty line



Comment at: flang/include/flang/Frontend/FrontendOptions.h:19
+enum ActionKind {
+  // This is temporary solution
+  // Avoids Action = InputOutputTest as option 0

Temporary solution to what?



Comment at: flang/include/flang/Frontend/FrontendOptions.h:56-59
+  Fortran77,
+  Fortran90,
+  Fortran95,
+  FortranF18

Could we defer adding this to later? This patch is already rather larger.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:72
 dashX = llvm::StringSwitch(XValue)
-.Case("f90", Language::Fortran)
+.Case("f90", Language::Fortran90)
 .Default(Language::Unknown);

Please, could we defer this to later?



Comment at: flang/lib/Frontend/FrontendOptions.cpp:18-23
+  .Cases("ff", "fpp", "FPP", "cuf", "CUF", "fir", "FOR", "for",
+ Language::Fortran)
+  .Cases("f", "F", "f77", Language::Fortran77)
+  .Cases("f90", "F90", ".ff90", Language::Fortran90)
+  .Cases("f95", "F95", "ff95", Language::Fortran95)
+  .Cases("f18", "F18", Language::FortranF18)

Could you reduce it to `Language::Fortran` only? We can deal with various 
variants in a separate patch when it becomes relevant.



Comment at: flang/test/Flang-Driver/driver-help.f90:14
+
 ! REQUIRES: new-flang-driver
 

Probably should be at the top (for consistency with driver-help-hidden.f90)



Comment at: flang/test/Flang-Driver/driver-help.f90:22
+! CHECK-FLANG-NEXT:OPTIONS:
+! CHECK-FLANG-NEXT: -help Display available options
+! CHECK-FLANG-NEXT: --version Print version information

What about `-o`?



Comment at: flang/test/Flang-Driver/emit-obj.f90:2
 ! RUN: not %flang-new  %s 2>&1 | FileCheck %s --check-prefix=ERROR-IMPLICIT
-! RUN: not %flang-new  -emit-obj %s 2>&1 | FileCheck %s 
--check-prefix=ERROR-EXPLICIT
 ! RUN: not %flang-new  -fc1 -emit-obj %s 2>&1 | FileCheck %s 
--check-prefix=ERROR-FC1

CarolineConcatto wrote:
> awarzynski wrote:
> > Why is this line deleted? `flang-new -emit-obj` should still fail, right?
> I remove this because the text does not apply any more
> ! ERROR-IMPLICIT: error: unknown 

[PATCH] D88694: [CMake][Fuchsia] Don't set WIN32 API, rely on autodetection

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.
phosek added a reviewer: leonardchan.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.
phosek requested review of this revision.

We prefer autodetection here to avoid persisting this configuration
in the generated __config header which is shared across targets.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88694

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -84,7 +84,6 @@
   set(RUNTIMES_${target}_CMAKE_SYSTEM_NAME Windows CACHE STRING "")
   set(RUNTIMES_${target}_CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "")
   set(RUNTIMES_${target}_LIBCXX_ABI_VERSION 2 CACHE STRING "")
-  set(RUNTIMES_${target}_LIBCXX_HAS_WIN32_THREAD_API ON CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_FILESYSTEM OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_ABI_LINKER_SCRIPT OFF CACHE BOOL "")


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -84,7 +84,6 @@
   set(RUNTIMES_${target}_CMAKE_SYSTEM_NAME Windows CACHE STRING "")
   set(RUNTIMES_${target}_CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "")
   set(RUNTIMES_${target}_LIBCXX_ABI_VERSION 2 CACHE STRING "")
-  set(RUNTIMES_${target}_LIBCXX_HAS_WIN32_THREAD_API ON CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_FILESYSTEM OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_ABI_LINKER_SCRIPT OFF CACHE BOOL "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:33
+
+  for (const FunctionDecl *D : Node.redecls())
+if (D->getASTContext().getSourceManager().isInSystemHeader(

I'm not certain I understand why we're looking through the entire redeclaration 
chain to see if the function is ever mentioned in a system header. I was 
expecting we'd look at the expansion location of the declaration and see if 
that's in a system header, which is already handled by the 
`isExpansionInSystemHeader()` matcher. Similar below.



Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:84
+  functionDecl(hasName(SignalFun), parameterCountIs(2),
+   anyOf(isInStdNamespace(), isTopLevelSystemFunction()));
+  auto HandlerExpr =

I think this can be simplified to remove the `anyOf`:
```
functionDecl(hasAnyName("::signal", "::std::signal"), 
isExpansionInSystemHeader())
```
should be sufficient (if there's a function named `signal` in a system header 
that has the wrong parameter count, I'd be surprised).



Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:97
+void SignalHandlerCheck::check(const MatchFinder::MatchResult ) {
+  auto *SignalCall = Result.Nodes.getNodeAs("register_call");
+  auto *HandlerDecl = Result.Nodes.getNodeAs("handler_decl");

All of these should be `const auto *` (wow, the lint pre-merge check was 
actually useful for once!)



Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:165
+  const IdentifierInfo *II = FD->getIdentifier();
+  // Nonnamed functions are not explicitly allowed.
+  if (!II)

How about: `Unnamed functions are explicitly not allowed.`



Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:171
+return true;
+
+  return false;

I think a configuration option is needed for users to be able to add their own 
conforming functions and by default, that list should include the functions 
that POSIX specifies as being async signal safe (at least on POSIX systems, 
similar for Windows if Microsoft documents such a list).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87449

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

[PATCH] D88238: [APFloat] convert SNaN to QNaN in convert() and raise Invalid signal

[Sanjay Patel via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG149f5b573c79: [APFloat] convert SNaN to QNaN in convert() 
and raise Invalid signal (authored by spatel).
Herald added subscribers: cfe-commits, jrtc27.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88238

Files:
  clang/test/CodeGen/builtin-nan-exception.c
  clang/test/CodeGen/builtin-nan-legacy.c
  clang/test/CodeGen/mips-unsupported-nan.c
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/Support/APFloat.cpp
  llvm/test/Transforms/InstSimplify/ConstProp/cast.ll
  llvm/test/Transforms/PhaseOrdering/X86/nancvt.ll
  llvm/unittests/ADT/APFloatTest.cpp

Index: llvm/unittests/ADT/APFloatTest.cpp
===
--- llvm/unittests/ADT/APFloatTest.cpp
+++ llvm/unittests/ADT/APFloatTest.cpp
@@ -1816,11 +1816,12 @@
   EXPECT_FALSE(losesInfo);
 
   test = APFloat::getSNaN(APFloat::IEEEsingle());
-  APFloat X87SNaN = APFloat::getSNaN(APFloat::x87DoubleExtended());
   APFloat::opStatus status = test.convert(APFloat::x87DoubleExtended(), APFloat::rmNearestTiesToEven, );
-  EXPECT_TRUE(test.bitwiseIsEqual(X87SNaN));
+  // Conversion quiets the SNAN, so now 2 bits of the 64-bit significand should be set.
+  APInt topTwoBits(64, 0x6000);
+  EXPECT_TRUE(test.bitwiseIsEqual(APFloat::getQNaN(APFloat::x87DoubleExtended(), false, )));
   EXPECT_FALSE(losesInfo);
-  EXPECT_EQ(status, APFloat::opOK);
+  EXPECT_EQ(status, APFloat::opInvalidOp);
 
   test = APFloat::getQNaN(APFloat::IEEEsingle());
   APFloat X87QNaN = APFloat::getQNaN(APFloat::x87DoubleExtended());
@@ -1832,6 +1833,7 @@
   test = APFloat::getSNaN(APFloat::x87DoubleExtended());
   test.convert(APFloat::x87DoubleExtended(), APFloat::rmNearestTiesToEven,
);
+  APFloat X87SNaN = APFloat::getSNaN(APFloat::x87DoubleExtended());
   EXPECT_TRUE(test.bitwiseIsEqual(X87SNaN));
   EXPECT_FALSE(losesInfo);
 
@@ -1841,13 +1843,13 @@
   EXPECT_TRUE(test.bitwiseIsEqual(X87QNaN));
   EXPECT_FALSE(losesInfo);
 
-  // The payload is lost in truncation, but we must retain NaN, so we set the bit after the quiet bit.
+  // The payload is lost in truncation, but we retain NaN by setting the quiet bit.
   APInt payload(52, 1);
   test = APFloat::getSNaN(APFloat::IEEEdouble(), false, );
   status = test.convert(APFloat::IEEEsingle(), APFloat::rmNearestTiesToEven, );
-  EXPECT_EQ(0x7fa0, test.bitcastToAPInt());
+  EXPECT_EQ(0x7fc0, test.bitcastToAPInt());
   EXPECT_TRUE(losesInfo);
-  EXPECT_EQ(status, APFloat::opOK);
+  EXPECT_EQ(status, APFloat::opInvalidOp);
 
   // The payload is lost in truncation. QNaN remains QNaN.
   test = APFloat::getQNaN(APFloat::IEEEdouble(), false, );
Index: llvm/test/Transforms/PhaseOrdering/X86/nancvt.ll
===
--- llvm/test/Transforms/PhaseOrdering/X86/nancvt.ll
+++ llvm/test/Transforms/PhaseOrdering/X86/nancvt.ll
@@ -18,6 +18,9 @@
 
 @var = external global i32
 
+; SNAN becomes QNAN on fptrunc:
+; 2147228864 = 0x7ffc1cc0 : QNAN
+
 define i32 @main() {
 ; CHECK-LABEL: @main(
 ; CHECK-NEXT:  entry:
@@ -30,15 +33,15 @@
 ; CHECK-NEXT:store volatile i32 2147228864, i32* @var, align 4
 ; CHECK-NEXT:store volatile i32 2147228864, i32* @var, align 4
 ; CHECK-NEXT:store volatile i32 2147228864, i32* @var, align 4
-; CHECK-NEXT:store volatile i32 2146502828, i32* @var, align 4
+; CHECK-NEXT:store volatile i32 2147027116, i32* @var, align 4
 ; CHECK-NEXT:store volatile i32 -1610612736, i32* @var, align 4
-; CHECK-NEXT:store volatile i32 2146502828, i32* @var, align 4
+; CHECK-NEXT:store volatile i32 2147027116, i32* @var, align 4
 ; CHECK-NEXT:store volatile i32 -2147483648, i32* @var, align 4
-; CHECK-NEXT:store volatile i32 2146502828, i32* @var, align 4
+; CHECK-NEXT:store volatile i32 2147027116, i32* @var, align 4
 ; CHECK-NEXT:store volatile i32 -1073741824, i32* @var, align 4
-; CHECK-NEXT:store volatile i32 2143034560, i32* @var, align 4
-; CHECK-NEXT:store volatile i32 2143034560, i32* @var, align 4
-; CHECK-NEXT:store volatile i32 2143034560, i32* @var, align 4
+; CHECK-NEXT:store volatile i32 2147228864, i32* @var, align 4
+; CHECK-NEXT:store volatile i32 2147228864, i32* @var, align 4
+; CHECK-NEXT:store volatile i32 2147228864, i32* @var, align 4
 ; CHECK-NEXT:ret i32 undef
 ;
 entry:
Index: llvm/test/Transforms/InstSimplify/ConstProp/cast.ll
===
--- llvm/test/Transforms/InstSimplify/ConstProp/cast.ll
+++ llvm/test/Transforms/InstSimplify/ConstProp/cast.ll
@@ -40,24 +40,24 @@
 }
 
 ; https://llvm.org/PR43907 - make sure that NaN doesn't morph into Inf.
-; SNaN remains SNaN.
+; SNaN becomes QNaN.
 
 define float @nan_f64_trunc() {
 ; 

[clang] 149f5b5 - [APFloat] convert SNaN to QNaN in convert() and raise Invalid signal

[Sanjay Patel via cfe-commits]

Author: Sanjay Patel
Date: 2020-10-01T14:37:38-04:00
New Revision: 149f5b573c79eac0c519ada4d2f7c50e17796cdf

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

LOG: [APFloat] convert SNaN to QNaN in convert() and raise Invalid signal

This is an alternate fix (see D87835) for a bug where a NaN constant
gets wrongly transformed into Infinity via truncation.
In this patch, we uniformly convert any SNaN to QNaN while raising
'invalid op'.
But we don't have a way to directly specify a 32-bit SNaN value in LLVM IR,
so those are always encoded/decoded by calling convert from/to 64-bit hex.

See D88664 for a clang fix needed to allow this change.

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

Added: 


Modified: 
clang/test/CodeGen/builtin-nan-exception.c
clang/test/CodeGen/builtin-nan-legacy.c
clang/test/CodeGen/mips-unsupported-nan.c
llvm/lib/AsmParser/LLParser.cpp
llvm/lib/IR/AsmWriter.cpp
llvm/lib/Support/APFloat.cpp
llvm/test/Transforms/InstSimplify/ConstProp/cast.ll
llvm/test/Transforms/PhaseOrdering/X86/nancvt.ll
llvm/unittests/ADT/APFloatTest.cpp

Removed: 




diff  --git a/clang/test/CodeGen/builtin-nan-exception.c 
b/clang/test/CodeGen/builtin-nan-exception.c
index a0de25e52ebe6..7445411ddf89e 100644
--- a/clang/test/CodeGen/builtin-nan-exception.c
+++ b/clang/test/CodeGen/builtin-nan-exception.c
@@ -17,8 +17,12 @@ float f[] = {
 
 
 // Doubles are created and converted to floats.
+// Converting (truncating) to float quiets the NaN (sets the MSB
+// of the significand) and raises the APFloat invalidOp exception
+// but that should not cause a compilation error in the default
+// (ignore FP exceptions) mode.
 
-// CHECK: float 0x7FF8, float 0x7FF4
+// CHECK: float 0x7FF8, float 0x7FFC
 
 float converted_to_float[] = {
   __builtin_nan(""),

diff  --git a/clang/test/CodeGen/builtin-nan-legacy.c 
b/clang/test/CodeGen/builtin-nan-legacy.c
index cd0f0fd14f14c..de6c15379a4dd 100644
--- a/clang/test/CodeGen/builtin-nan-legacy.c
+++ b/clang/test/CodeGen/builtin-nan-legacy.c
@@ -1,7 +1,15 @@
 // RUN: %clang -target mipsel-unknown-linux -mnan=legacy -emit-llvm -S %s -o - 
| FileCheck %s
-// CHECK: float 0x7FF4, float 0x7FF8
+// CHECK: float 0x7FFC, float 0x7FF8
 // CHECK: double 0x7FF4, double 0x7FF8
 
+// The first line shows an unintended consequence.
+// __builtin_nan() creates a legacy QNAN double with an empty payload
+// (the first bit of the significand is clear to indicate quiet, so
+// the second bit of the payload is set to maintain NAN-ness).
+// The value is then truncated, but llvm::APFloat does not know about
+// the inverted quiet bit, so it sets the first bit on conversion
+// to indicate 'quiet' independently of the setting in clang.
+
 float f[] = {
   __builtin_nan(""),
   __builtin_nans(""),

diff  --git a/clang/test/CodeGen/mips-unsupported-nan.c 
b/clang/test/CodeGen/mips-unsupported-nan.c
index 2fd5042e92f8e..16cea3c2e7e18 100644
--- a/clang/test/CodeGen/mips-unsupported-nan.c
+++ b/clang/test/CodeGen/mips-unsupported-nan.c
@@ -39,7 +39,21 @@
 // CHECK-MIPS64: warning: ignoring '-mnan=2008' option because the 'mips64' 
architecture does not support it
 // CHECK-MIPS64R6: warning: ignoring '-mnan=legacy' option because the 
'mips64r6' architecture does not support it
 
-// CHECK-NANLEGACY: float 0x7FF4
+// This call creates a QNAN double with an empty payload.
+// The quiet bit is inverted in legacy mode: it is clear to indicate QNAN,
+// so the next highest bit is set to maintain NAN (not infinity).
+// In regular (2008) mode, the quiet bit is set to indicate QNAN.
+
+// CHECK-NANLEGACY: double 0x7FF4
+// CHECK-NAN2008: double 0x7FF8
+
+double d =  __builtin_nan("");
+
+// This call creates a QNAN double with an empty payload and then truncates.
+// llvm::APFloat does not know about the inverted quiet bit, so it sets the
+// quiet bit on conversion independently of the setting in clang.
+
+// CHECK-NANLEGACY: float 0x7FFC
 // CHECK-NAN2008: float 0x7FF8
 
 float f =  __builtin_nan("");

diff  --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index 63f8531dbdced..4e1ae4faa4e19 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -5345,6 +5345,8 @@ bool LLParser::ConvertValIDToValue(Type *Ty, ValID , 
Value *,
 // The lexer has no type info, so builds all half, bfloat, float, and 
double
 // FP constants as double.  Fix this here.  Long double does not need this.
 if (() == ::IEEEdouble()) {
+  // Check for signaling before potentially converting and losing that 
info.
+ 

[PATCH] D88345: [CUDA] Allow local `static const {__constant__, __device__}` variables.

[Artem Belevich via Phabricator via cfe-commits]

tra updated this revision to Diff 295636.
tra added a comment.
Herald added a reviewer: jdoerfert.

Fixed a test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88345

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenCUDA/static-device-var-no-rdc.cu
  clang/test/SemaCUDA/bad-attributes.cu
  clang/test/SemaCUDA/device-var-init.cu

Index: clang/test/SemaCUDA/device-var-init.cu
===
--- clang/test/SemaCUDA/device-var-init.cu
+++ clang/test/SemaCUDA/device-var-init.cu
@@ -196,34 +196,212 @@
 __constant__ T_FA_NED c_t_fa_ned;
 // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
 
-// Verify that only __shared__ local variables may be static on device
-// side and that they are not allowed to be initialized.
+// Verify that local variables may be static on device
+// side and that they conform to the initialization constraints.
+// __shared__ can't be initialized at all and others don't support dynamic initialization.
 __device__ void df_sema() {
-  static __shared__ NCFS s_ncfs;
-  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
-  static __shared__ UC s_uc;
-  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
-  static __shared__ NED s_ned;
-  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
-
   static __device__ int ds;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static __constant__ int dc;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static int v;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static const int cv = 1;
   static const __device__ int cds = 1;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static const __constant__ int cdc = 1;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
+
+
+  // __shared__ does not need to be explicitly static.
+  __shared__ int lsi;
+  // __constant__ and __device__ can not be non-static local
+  __constant__ int lci;
+  // expected-error@-1 {{__constant__ and __device__ are not allowed on non-static local variables}}
+  __device__ int ldi;
+  // expected-error@-1 {{__constant__ and __device__ are not allowed on non-static local variables}}
+  
+  // Same test cases as for the globals above.
+
+  static __device__ int d_v_f = f();
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ int s_v_f = f();
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static __constant__ int c_v_f = f();
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
+  static __shared__ T s_t_i = {2};
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+
+  static __device__ EC d_ec_i(3);
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ EC s_ec_i(3);
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static __constant__ EC c_ec_i(3);
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
+  static __device__ EC d_ec_i2 = {3};
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ EC s_ec_i2 = {3};
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static __constant__ EC c_ec_i2 = {3};
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
+  static __device__ ETC d_etc_i(3);
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ ETC s_etc_i(3);
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static __constant__ ETC c_etc_i(3);
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
+  static __device__ ETC 

[PATCH] D88345: [CUDA] Allow local `static const {__constant__, __device__}` variables.

[Artem Belevich via Phabricator via cfe-commits]

tra updated this revision to Diff 295635.
tra retitled this revision from "[CUDA] Allow local `static {__constant__, 
__device__}` variables." to "[CUDA] Allow local `static const {__constant__, 
__device__}` variables.".
tra edited the summary of this revision.
tra added a comment.
Herald added a reviewer: aaron.ballman.

Further relaxed application of attributes on local static variables.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88345

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenCUDA/static-device-var-no-rdc.cu
  clang/test/SemaCUDA/device-var-init.cu

Index: clang/test/SemaCUDA/device-var-init.cu
===
--- clang/test/SemaCUDA/device-var-init.cu
+++ clang/test/SemaCUDA/device-var-init.cu
@@ -196,34 +196,212 @@
 __constant__ T_FA_NED c_t_fa_ned;
 // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
 
-// Verify that only __shared__ local variables may be static on device
-// side and that they are not allowed to be initialized.
+// Verify that local variables may be static on device
+// side and that they conform to the initialization constraints.
+// __shared__ can't be initialized at all and others don't support dynamic initialization.
 __device__ void df_sema() {
-  static __shared__ NCFS s_ncfs;
-  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
-  static __shared__ UC s_uc;
-  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
-  static __shared__ NED s_ned;
-  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
-
   static __device__ int ds;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static __constant__ int dc;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static int v;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static const int cv = 1;
   static const __device__ int cds = 1;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static const __constant__ int cdc = 1;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
+
+
+  // __shared__ does not need to be explicitly static.
+  __shared__ int lsi;
+  // __constant__ and __device__ can not be non-static local
+  __constant__ int lci;
+  // expected-error@-1 {{__constant__ and __device__ are not allowed on non-static local variables}}
+  __device__ int ldi;
+  // expected-error@-1 {{__constant__ and __device__ are not allowed on non-static local variables}}
+  
+  // Same test cases as for the globals above.
+
+  static __device__ int d_v_f = f();
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ int s_v_f = f();
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static __constant__ int c_v_f = f();
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
+  static __shared__ T s_t_i = {2};
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+
+  static __device__ EC d_ec_i(3);
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ EC s_ec_i(3);
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static __constant__ EC c_ec_i(3);
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
+  static __device__ EC d_ec_i2 = {3};
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ EC s_ec_i2 = {3};
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static __constant__ EC c_ec_i2 = {3};
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
+  static __device__ ETC d_etc_i(3);
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ ETC s_etc_i(3);
+  // expected-error@-1 

[PATCH] D77062: [analyzer] Improve zero assumption in CStringChecke::assumeZero

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

In D77062#2305856 , @ASDenysPetrov 
wrote:

> @steakhal
>
>> @ASDenysPetrov Do you still want to rework the API of the `assumeZero`?
>
> This patch more looks like NFC, being just refactored.

Fine.

> Actually I see that if we find and fix the root cause, we can freely refuse 
> this patch.
> Another thing I see is that this patch will work after a root cause fix as 
> well.

Yes, I think so.

> And the last one is that as long a root cause stays unfixed, clang will emit 
> an assertion without this patch, at least for my testcase.
>
> So, I can't really evaluate this objectively. What do you think?

Actually, I'm expecting to fix this in the need future. I'm really doing the 
extra mile to come up with a proper fix.
Unfortunately, it's deeply in the Core and requires a bunch of stuff to work 
out how to achieve this.
It will be a nice first-time experience though - I've already learned a lot 
just by debugging it.

Till then, I recommend you to follow my effort at D88477 
.


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

https://reviews.llvm.org/D77062

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

[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

In D88477#2304708 , @NoQ wrote:

> I'm trying to say that the value produced by the load should not be the same 
> as the stored value, because these two values are of different types. When 
> exactly does the first value change into the second value is a different 
> story; the current grand vision around which the code is written says that it 
> changes during load, therefore it's the load code (step #2) that's to blame.

Are you implying that the second dereference of `b` should produce an lvalue of 
the type `char *`, instead of the type of the SVal 
`{SymRegion{reg_$0},0 S64b,unsigned char}`.
So, I should do this cast when we evaluate the dereference, and produce an 
lvalue of the static type, aka binding the SVal `{SymRegion{reg_$0},0 S64b,char*}` to it.
In the AST, it is the line `@1`:

  `-IfStmt
-BinaryOperator 'bool' '=='
 |-ImplicitCastExpr 'char *' 
  @1:| `-UnaryOperator 'char *' lvalue prefix '*' cannot overflow
 |   `-ImplicitCastExpr 'char **' 
 | `-UnaryOperator 'char **' lvalue prefix '*' cannot overflow
 |   `-ImplicitCastExpr 'char ***' 
 | `-DeclRefExpr 'char ***' lvalue ParmVar 0x55e808fe8188 'b' 'char 
***'
 | `-ImplicitCastExpr 'char *' 
  `-IntegerLiteral 'int' 0

To do this, I would have to modify the `ExprEngine::handleUOExtension` to not 
just simply lookup the corresponding SVal of the Environment's ExprBindings, 
but also apply the cast if necessary.
Am I on the right track now?

PS: I tried to implement this, but I failed to reuse the other overloads of 
`getSVal` to accomplish this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88477

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

[clang] c1b209c - [Format] Don't treat compound extension headers (foo.proto.h) as foo.cc main-file header.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-10-01T19:57:57+02:00
New Revision: c1b209cc61290f1ce1243470b825e0994645cb7d

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

LOG: [Format] Don't treat compound extension headers (foo.proto.h) as foo.cc 
main-file header.

We receive internal bugs about this false positives after D86597.

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

Added: 


Modified: 
clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
clang/unittests/Format/SortIncludesTest.cpp

Removed: 




diff  --git a/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp 
b/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
index e0368975ea3e..0cc4afa4ade6 100644
--- a/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ b/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -233,7 +233,12 @@ int 
IncludeCategoryManager::getSortIncludePriority(StringRef IncludeName,
 bool IncludeCategoryManager::isMainHeader(StringRef IncludeName) const {
   if (!IncludeName.startswith("\""))
 return false;
-  StringRef HeaderStem = matchingStem(IncludeName.drop_front(1).drop_back(1));
+
+  // Not matchingStem: implementation files may have compound extensions but
+  // headers may not.
+  StringRef HeaderStem =
+  llvm::sys::path::stem(IncludeName.drop_front(1).drop_back(
+  1) /* remove the surrounding "" or <> */);
   if (FileStem.startswith(HeaderStem) ||
   FileStem.startswith_lower(HeaderStem)) {
 llvm::Regex MainIncludeRegex(HeaderStem.str() + Style.IncludeIsMainRegex,

diff  --git a/clang/unittests/Format/SortIncludesTest.cpp 
b/clang/unittests/Format/SortIncludesTest.cpp
index db3ed65d443b..c327be5e6b0b 100644
--- a/clang/unittests/Format/SortIncludesTest.cpp
+++ b/clang/unittests/Format/SortIncludesTest.cpp
@@ -151,6 +151,16 @@ TEST_F(SortIncludesTest, NoReplacementsForValidIncludes) {
   EXPECT_TRUE(sortIncludes(FmtStyle, Code, GetCodeRange(Code), 
"a.cc").empty());
 }
 
+TEST_F(SortIncludesTest, NoMainFileHeader) {
+  std::string Code = "#include \n"
+ "\n"
+ "#include \"a/extra_action.proto.h\"\n";
+  FmtStyle = getGoogleStyle(FormatStyle::LK_Cpp);
+  EXPECT_TRUE(
+  sortIncludes(FmtStyle, Code, GetCodeRange(Code), "a/extra_action.cc")
+  .empty());
+}
+
 TEST_F(SortIncludesTest, SortedIncludesInMultipleBlocksAreMerged) {
   Style.IncludeBlocks = tooling::IncludeStyle::IBS_Merge;
   EXPECT_EQ("#include \"a.h\"\n"



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

[PATCH] D63640: [clang] Improve Serialization/Imporing of APValues

[Tyker via Phabricator via cfe-commits]

Tyker added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:9010
+  ToPath[Idx] =
+  cast(const_cast(ImpDecl.get()));
+}

rsmith wrote:
> We want the path in an `APValue` to be canonical, but importing a canonical 
> decl might result in a non-canonical decl.
> but importing a canonical decl might result in a non-canonical decl.
this is a quite surprising behavior.



Comment at: clang/lib/AST/ASTImporter.cpp:9030-9031
+} else {
+  FromElemTy =
+  FromValue.getLValueBase().get()->getType();
+  llvm::Expected ImpDecl =

rsmith wrote:
> If you're intentionally not handling `DynamicAllocLValue` here (because those 
> should always be transient), a comment to that effect would be useful.
i added an asserts with a message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63640

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

[PATCH] D63640: [clang] Improve Serialization/Imporing of APValues

[Tyker via Phabricator via cfe-commits]

Tyker updated this revision to Diff 295626.
Tyker marked 15 inline comments as done.
Tyker added a comment.

addressed comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63640

Files:
  clang/include/clang/AST/APValue.h
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/ASTImporter.h
  clang/lib/AST/APValue.cpp
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/ASTMerge/APValue/APValue.cpp

Index: clang/test/ASTMerge/APValue/APValue.cpp
===
--- /dev/null
+++ clang/test/ASTMerge/APValue/APValue.cpp
@@ -0,0 +1,460 @@
+// RUN: %clang_cc1 -std=gnu++2a -emit-pch %s -o %t.pch
+// RUN: %clang_cc1 -std=gnu++2a %s -DEMIT -ast-merge %t.pch -ast-dump-all | FileCheck %s
+
+#ifndef EMIT
+#define EMIT
+
+namespace Integer {
+
+consteval int fint() {
+  return 6789;
+}
+
+int Unique_Int = fint();
+//CHECK:  VarDecl {{.*}} Unique_Int
+//CHECK-NEXT: ConstantExpr {{.*}} 'int'
+//CHECK-NEXT: value: Int 6789
+
+consteval __uint128_t fint128() {
+  return ((__uint128_t)0x75f17d6b3588f843 << 64) | 0xb13dea7c9c324e51;
+}
+
+constexpr __uint128_t Unique_Int128 = fint128();
+//CHECK:  VarDecl {{.*}} Unique_Int128
+//CHECK-NEXT: value: Int 156773562844924187900898496343692168785
+//CHECK-NEXT: ConstantExpr
+//CHECK-NEXT: value: Int 156773562844924187900898496343692168785
+
+} // namespace Integer
+
+namespace FloatingPoint {
+
+consteval double fdouble() {
+  return double(567890.67890);
+}
+
+double Unique_Double = fdouble();
+//CHECK:  VarDecl {{.*}} Unique_Double
+//CHECK-NEXT: ConstantExpr {{.*}}
+//CHECK-NEXT: value: Float 5.678907e+05
+
+} // namespace FloatingPoint
+
+// FIXME: Add test for FixedPoint, ComplexInt, ComplexFloat, AddrLabelDiff.
+
+namespace Struct {
+
+struct B {
+  int i;
+  double d;
+};
+
+consteval B fB() {
+  return B{1, 0.7};
+}
+
+constexpr B Basic_Struct = fB();
+//CHECK:  VarDecl {{.*}} Basic_Struct
+//CHECK-NEXT: value: Struct
+//CHECK-NEXT: fields: Int 1, Float 7.00e-01
+//CHECK-NEXT: ImplicitCastExpr
+//CHECK-NEXT: ConstantExpr
+//CHECK-NEXT: value: Struct
+//CHECK-NEXT: fields: Int 1, Float 7.00e-01
+
+struct C {
+  int i = 9;
+};
+
+struct A : B {
+  constexpr A(B b, int I, double D, C _c) : B(b), i(I), d(D), c(_c) {}
+  int i;
+  double d;
+  C c;
+};
+
+consteval A fA() {
+  return A(Basic_Struct, 1, 79.789, {});
+}
+
+A Advanced_Struct = fA();
+//CHECK:  VarDecl {{.*}} Advanced_Struct
+//CHECK-NEXT: ConstantExpr {{.*}}
+//CHECK-NEXT: value: Struct
+//CHECK-NEXT: base: Struct
+//CHECK-NEXT: fields: Int 1, Float 7.00e-01
+//CHECK-NEXT: fields: Int 1, Float 7.978900e+01
+//CHECK-NEXT: field: Struct
+//CHECK-NEXT: field: Int 9
+
+} // namespace Struct
+
+namespace Vector {
+
+using v4si = int __attribute__((__vector_size__(16)));
+
+consteval v4si fv4si() {
+  return (v4si){8, 2, 3};
+}
+
+v4si Vector_Int = fv4si();
+//CHECK:  VarDecl {{.*}} Vector_Int
+//CHECK-NEXT: ConstantExpr
+//CHECK-NEXT: value: Vector length=4
+//CHECK-NEXT: elements: Int 8, Int 2, Int 3, Int 0
+
+} // namespace Vector
+
+namespace Array {
+
+struct B {
+  int arr[6];
+};
+
+consteval B fint() {
+  return B{1, 2, 3, 4, 5, 6};
+}
+
+B Array_Int = fint();
+//CHECK:  VarDecl {{.*}} Array_Int
+//CHECK-NEXT: ConstantExpr
+//CHECK-NEXT: value: Struct
+//CHECK-NEXT: field: Array size=6
+//CHECK-NEXT: elements: Int 1, Int 2, Int 3, Int 4
+//CHECK-NEXT: elements: Int 5, Int 6
+
+struct A {
+  int i = 789;
+  double d = 67890.09876;
+};
+
+struct C {
+  A arr[3];
+};
+
+consteval C fA() {
+  return {{A{}, A{-45678, 9.8}, A{9}}};
+}
+
+C Array2_Struct = fA();
+//CHECK:  VarDecl {{.*}} Array2_Struct
+//CHECK-NEXT: ConstantExpr {{.*}}
+
+using v4si = int __attribute__((__vector_size__(16)));
+
+struct D {
+  v4si arr[2];
+};
+
+consteval D fv4si() {
+  return {{{1, 2, 3, 4}, {4, 5, 6, 7}}};
+}
+
+D Array_Vector = fv4si();
+//CHECK:  VarDecl {{.*}} Array_Vector
+//CHECK-NEXT: ConstantExpr {{.*}}
+//CHECK-NEXT: value: Struct
+//CHECK-NEXT: field: Array size=2
+//CHECK-NEXT: element: Vector length=4
+//CHECK-NEXT: elements: Int 1, Int 2, Int 3, Int 4
+//CHECK-NEXT: element: Vector length=4
+//CHECK-NEXT: elements: Int 4, Int 5, Int 6, Int 7
+
+} // namespace Array
+
+namespace Union {
+
+struct A {
+  int i = 6789;
+  float f = 987.9876;
+};
+
+union U {
+  int i;
+  A a{567890, 9876.5678f};
+};
+
+consteval U fU1() {
+  return U{0};
+}
+
+U Unique_Union1 = fU1();
+//CHECK:  VarDecl {{.*}} Unique_Union
+//CHECK-NEXT: ConstantExpr
+//CHECK-NEXT: value: Union .i Int 0
+
+consteval U fU() {
+  return U{};
+}
+
+U Unique_Union2 = fU();
+//CHECK:  VarDecl {{.*}} Unique_Union
+//CHECK-NEXT: ConstantExpr
+//CHECK-NEXT: value: Union .a
+//CHECK-NEXT: Struct
+//CHECK-NEXT: fields: Int 567890, Float 

[PATCH] D88680: Add ability to turn off -fpch-instantiate-templates in clang-cl

[Shivanshu Goyal via Phabricator via cfe-commits]

shivanshu3 added a comment.

Note that I do not have commit access and this change will have to be committed 
by someone else on my behalf. Please use the following commit author details:
"Shivan Goyal "


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88680

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

[clang] 686eb0d - [AST] do not error on APFloat invalidOp in default mode

[Sanjay Patel via cfe-commits]

Author: Sanjay Patel
Date: 2020-10-01T13:46:45-04:00
New Revision: 686eb0d8ded9159b090c3ef7b33a422e1f05166e

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

LOG: [AST] do not error on APFloat invalidOp in default mode

If FP exceptions are ignored, we should not error out of compilation
just because APFloat indicated an exception.
This is required as a preliminary step for D88238
which changes APFloat behavior for signaling NaN convert() to set
the opInvalidOp exception status.

Currently, there is no way to trigger this error because convert()
never sets opInvalidOp. FP binops that set opInvalidOp also create
a NaN, so the path to checkFloatingPointResult() is blocked by a
different diagnostic:

  // [expr.pre]p4:
  //   If during the evaluation of an expression, the result is not
  //   mathematically defined [...], the behavior is undefined.
  // FIXME: C++ rules require us to not conform to IEEE 754 here.
  if (LHS.isNaN()) {
Info.CCEDiag(E, diag::note_constexpr_float_arithmetic) << LHS.isNaN();
return Info.noteUndefinedBehavior();
  }
  return checkFloatingPointResult(Info, E, St);

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

Added: 


Modified: 
clang/lib/AST/ExprConstant.cpp

Removed: 




diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index b17eed2dc823..4460e3a17e6d 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2439,7 +2439,8 @@ static bool checkFloatingPointResult(EvalInfo , 
const Expr *E,
 return false;
   }
 
-  if (St & APFloat::opStatus::opInvalidOp) {
+  if ((St & APFloat::opStatus::opInvalidOp) &&
+  FPO.getFPExceptionMode() != LangOptions::FPE_Ignore) {
 // There is no usefully definable result.
 Info.FFDiag(E);
 return false;



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

[PATCH] D88664: [AST] do not error on APFloat invalidOp in default mode

[Sanjay Patel via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG686eb0d8ded9: [AST] do not error on APFloat invalidOp in 
default mode (authored by spatel).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88664

Files:
  clang/lib/AST/ExprConstant.cpp


Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -2439,7 +2439,8 @@
 return false;
   }
 
-  if (St & APFloat::opStatus::opInvalidOp) {
+  if ((St & APFloat::opStatus::opInvalidOp) &&
+  FPO.getFPExceptionMode() != LangOptions::FPE_Ignore) {
 // There is no usefully definable result.
 Info.FFDiag(E);
 return false;


Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -2439,7 +2439,8 @@
 return false;
   }
 
-  if (St & APFloat::opStatus::opInvalidOp) {
+  if ((St & APFloat::opStatus::opInvalidOp) &&
+  FPO.getFPExceptionMode() != LangOptions::FPE_Ignore) {
 // There is no usefully definable result.
 Info.FFDiag(E);
 return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88680: Add ability to turn off -fpch-instantiate-templates in clang-cl

[Shivanshu Goyal via Phabricator via cfe-commits]

shivanshu3 created this revision.
shivanshu3 added reviewers: llunak, rnk, rsmith, hans, zahen.
shivanshu3 added a project: clang.
Herald added subscribers: cfe-commits, dang.
shivanshu3 requested review of this revision.

A lot of our code building with clang-cl.exe using Clang 11 was failing with 
the following 2 type of errors:

1. explicit specialization of 'foo' after instantiation
2. no matching function for call to 'bar'

Note that we also use `-fdelayed-template-parsing` in our builds.

I tried pretty hard to get a small repro for these failures, but couldn't. So 
there is some subtle edge case in the `-fpch-instantiate-templates` feature 
introduced by this change:
https://reviews.llvm.org/D69585

When I tried turning this off using `-fno-pch-instantiate-templates`, builds 
would silently fail with the same error without any indication that 
`-fno-pch-instantiate-templates` was being ignored by the compiler. Then I 
realized this "no" option wasn't actually working when I ran Clang under a 
debugger.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88680

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -1212,7 +1212,10 @@
 if (YcArg && JA.getKind() >= Action::PrecompileJobClass &&
 JA.getKind() <= Action::AssembleJobClass) {
   CmdArgs.push_back(Args.MakeArgString("-building-pch-with-obj"));
-  CmdArgs.push_back(Args.MakeArgString("-fpch-instantiate-templates"));
+  // -fpch-instantiate-templates is the default when creating
+  // precomp using /Yc
+  if (!Args.hasArg(options::OPT_fno_pch_instantiate_templates))
+CmdArgs.push_back(Args.MakeArgString("-fpch-instantiate-templates"));
 }
 if (YcArg || YuArg) {
   StringRef ThroughHeader = YcArg ? YcArg->getValue() : YuArg->getValue();
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1475,11 +1475,11 @@
   Group, Flags<[DriverOption]>;
 def fpch_instantiate_templates:
   Flag <["-"], "fpch-instantiate-templates">,
-  Group, Flags<[CC1Option]>,
+  Group, Flags<[CC1Option, CoreOption]>,
   HelpText<"Instantiate templates already while building a PCH">;
 def fno_pch_instantiate_templates:
   Flag <["-"], "fno-pch-instantiate-templates">,
-  Group, Flags<[CC1Option]>;
+  Group, Flags<[CC1Option, CoreOption]>;
 defm pch_codegen: OptInFFlag<"pch-codegen", "Generate ", "Do not generate ",
   "code for uses of this PCH that assumes an explicit object file will be 
built for the PCH">;
 defm pch_debuginfo: OptInFFlag<"pch-debuginfo", "Generate ", "Do not generate 
",


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -1212,7 +1212,10 @@
 if (YcArg && JA.getKind() >= Action::PrecompileJobClass &&
 JA.getKind() <= Action::AssembleJobClass) {
   CmdArgs.push_back(Args.MakeArgString("-building-pch-with-obj"));
-  CmdArgs.push_back(Args.MakeArgString("-fpch-instantiate-templates"));
+  // -fpch-instantiate-templates is the default when creating
+  // precomp using /Yc
+  if (!Args.hasArg(options::OPT_fno_pch_instantiate_templates))
+CmdArgs.push_back(Args.MakeArgString("-fpch-instantiate-templates"));
 }
 if (YcArg || YuArg) {
   StringRef ThroughHeader = YcArg ? YcArg->getValue() : YuArg->getValue();
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1475,11 +1475,11 @@
   Group, Flags<[DriverOption]>;
 def fpch_instantiate_templates:
   Flag <["-"], "fpch-instantiate-templates">,
-  Group, Flags<[CC1Option]>,
+  Group, Flags<[CC1Option, CoreOption]>,
   HelpText<"Instantiate templates already while building a PCH">;
 def fno_pch_instantiate_templates:
   Flag <["-"], "fno-pch-instantiate-templates">,
-  Group, Flags<[CC1Option]>;
+  Group, Flags<[CC1Option, CoreOption]>;
 defm pch_codegen: OptInFFlag<"pch-codegen", "Generate ", "Do not generate ",
   "code for uses of this PCH that assumes an explicit object file will be built for the PCH">;
 defm pch_debuginfo: OptInFFlag<"pch-debuginfo", "Generate ", "Do not generate ",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88524: [CUDA][HIP] Fix bound arch for offload action for fat binary

[Artem Belevich via Phabricator via cfe-commits]

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



Comment at: clang/lib/Basic/Cuda.cpp:95
 const char *CudaArchToString(CudaArch A) {
+  if (A == CudaArch::UNUSED)
+return "";

You could add a `{CudaArch::UNUSED, "", ""}` to the table above instead of 
explicit checks here.


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

https://reviews.llvm.org/D88524

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

[PATCH] D88278: [PowerPC] Add builtins for xvtdiv(dp|sp) and xvtsqrt(dp|sp).

[Amy Kwan via Phabricator via cfe-commits]

amyk accepted this revision as: amyk.
amyk added a comment.
This revision is now accepted and ready to land.

Overall I think this LGTM.

Please correct me if I am wrong but I think the description of the functions 
need to be updated to:

  int vec_test_swdiv(vector double v1, vector double v2);
  int vec_test_swdivs(vector float v1, vector float v2);
  int vec_test_swsqrt(vector double v1);
  int vec_test_swsqrts(vector float v1);




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88278

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

[PATCH] D83144: Allow to specify macro names for android-comparison-in-temp-failure-retry.

[George Burgess IV via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9d40fb808fd0: Allow to specify macro names for 
android-comparison-in-temp-failure-retry (authored by fmayer, committed by 
george.burgess.iv).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83144

Files:
  clang-tools-extra/clang-tidy/android/ComparisonInTempFailureRetryCheck.cpp
  clang-tools-extra/clang-tidy/android/ComparisonInTempFailureRetryCheck.h
  
clang-tools-extra/docs/clang-tidy/checks/android-comparison-in-temp-failure-retry.rst
  
clang-tools-extra/test/clang-tidy/checkers/android-comparison-in-temp-failure-retry-custom-macro.c

Index: clang-tools-extra/test/clang-tidy/checkers/android-comparison-in-temp-failure-retry-custom-macro.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/android-comparison-in-temp-failure-retry-custom-macro.c
@@ -0,0 +1,46 @@
+// RUN: %check_clang_tidy %s android-comparison-in-temp-failure-retry %t -- -config="{CheckOptions: [{key: android-comparison-in-temp-failure-retry.RetryMacros, value: 'MY_TEMP_FAILURE_RETRY,MY_OTHER_TEMP_FAILURE_RETRY'}]}"
+
+#define MY_TEMP_FAILURE_RETRY(x) \
+  ({ \
+typeof(x) __z;   \
+do   \
+  __z = (x); \
+while (__z == -1);   \
+__z; \
+  })
+
+#define MY_OTHER_TEMP_FAILURE_RETRY(x) \
+  ({   \
+typeof(x) __z; \
+do \
+  __z = (x);   \
+while (__z == -1); \
+__z;   \
+  })
+
+int foo();
+int bar(int a);
+
+void with_custom_macro() {
+  MY_TEMP_FAILURE_RETRY(foo());
+  MY_TEMP_FAILURE_RETRY(foo() == 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: top-level comparison in MY_TEMP_FAILURE_RETRY
+  MY_TEMP_FAILURE_RETRY((foo()));
+  MY_TEMP_FAILURE_RETRY((int)(foo() == 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: top-level comparison in MY_TEMP_FAILURE_RETRY
+  MY_TEMP_FAILURE_RETRY((bar(foo() == 1)));
+  MY_TEMP_FAILURE_RETRY((int)((bar(foo() == 1)) == 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:49: warning: top-level comparison in MY_TEMP_FAILURE_RETRY
+}
+
+void with_other_custom_macro() {
+  MY_OTHER_TEMP_FAILURE_RETRY(foo());
+  MY_OTHER_TEMP_FAILURE_RETRY(foo() == 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: top-level comparison in MY_OTHER_TEMP_FAILURE_RETRY
+  MY_OTHER_TEMP_FAILURE_RETRY((foo()));
+  MY_OTHER_TEMP_FAILURE_RETRY((int)(foo() == 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:43: warning: top-level comparison in MY_OTHER_TEMP_FAILURE_RETRY
+  MY_OTHER_TEMP_FAILURE_RETRY((bar(foo() == 1)));
+  MY_OTHER_TEMP_FAILURE_RETRY((int)((bar(foo() == 1)) == 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:55: warning: top-level comparison in MY_OTHER_TEMP_FAILURE_RETRY
+}
Index: clang-tools-extra/docs/clang-tidy/checks/android-comparison-in-temp-failure-retry.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/android-comparison-in-temp-failure-retry.rst
+++ clang-tools-extra/docs/clang-tidy/checks/android-comparison-in-temp-failure-retry.rst
@@ -34,3 +34,10 @@
   while (TEMP_FAILURE_RETRY(read(STDIN_FILENO, cs, sizeof(cs))) != 0) {
 // Do something with cs.
   }
+
+Options
+---
+
+.. option:: RetryMacros
+
+   A comma-separated list of the names of retry macros to be checked.
Index: clang-tools-extra/clang-tidy/android/ComparisonInTempFailureRetryCheck.h
===
--- clang-tools-extra/clang-tidy/android/ComparisonInTempFailureRetryCheck.h
+++ clang-tools-extra/clang-tidy/android/ComparisonInTempFailureRetryCheck.h
@@ -10,6 +10,9 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_COMPARISONINTEMPFAILURERETRYCHECK_H
 
 #include "../ClangTidyCheck.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include 
 
 namespace clang {
 namespace tidy {
@@ -22,10 +25,14 @@
 /// TEMP_FAILURE_RETRY is a macro provided by both glibc and Bionic.
 class ComparisonInTempFailureRetryCheck : public ClangTidyCheck {
 public:
-  ComparisonInTempFailureRetryCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  ComparisonInTempFailureRetryCheck(StringRef Name, ClangTidyContext *Context);
+  void storeOptions(ClangTidyOptions::OptionMap ) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
+
+private:
+  const std::string RawRetryList;
+  SmallVector RetryMacros;
 };
 
 } // namespace android
Index: clang-tools-extra/clang-tidy/android/ComparisonInTempFailureRetryCheck.cpp

[clang-tools-extra] 9d40fb8 - Allow to specify macro names for android-comparison-in-temp-failure-retry

[George Burgess IV via cfe-commits]

Author: Florian Mayer
Date: 2020-10-01T10:09:26-07:00
New Revision: 9d40fb808fd0fbd33eb3b50c20d7f402de5db91e

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

LOG: Allow to specify macro names for android-comparison-in-temp-failure-retry

Some projects do not use the TEMP_FAILURE_RETRY macro but define their
own one, as not to depend on glibc / Bionic details. By allowing the
user to override the list of macros, these projects can also benefit
from this check.

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

Added: 

clang-tools-extra/test/clang-tidy/checkers/android-comparison-in-temp-failure-retry-custom-macro.c

Modified: 
clang-tools-extra/clang-tidy/android/ComparisonInTempFailureRetryCheck.cpp
clang-tools-extra/clang-tidy/android/ComparisonInTempFailureRetryCheck.h

clang-tools-extra/docs/clang-tidy/checks/android-comparison-in-temp-failure-retry.rst

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/android/ComparisonInTempFailureRetryCheck.cpp 
b/clang-tools-extra/clang-tidy/android/ComparisonInTempFailureRetryCheck.cpp
index 188d44da51d81..c7b9896c64f81 100644
--- a/clang-tools-extra/clang-tidy/android/ComparisonInTempFailureRetryCheck.cpp
+++ b/clang-tools-extra/clang-tidy/android/ComparisonInTempFailureRetryCheck.cpp
@@ -18,32 +18,17 @@ namespace clang {
 namespace tidy {
 namespace android {
 
-namespace {
-AST_MATCHER(BinaryOperator, isRHSATempFailureRetryArg) {
-  if (!Node.getBeginLoc().isMacroID())
-return false;
-
-  const SourceManager  = Finder->getASTContext().getSourceManager();
-  if 
(!SM.isMacroArgExpansion(Node.getRHS()->IgnoreParenCasts()->getBeginLoc()))
-return false;
-
-  const LangOptions  = Finder->getASTContext().getLangOpts();
-  SourceLocation LocStart = Node.getBeginLoc();
-  while (LocStart.isMacroID()) {
-SourceLocation Invocation = SM.getImmediateMacroCallerLoc(LocStart);
-Token Tok;
-if (!Lexer::getRawToken(SM.getSpellingLoc(Invocation), Tok, SM, Opts,
-/*IgnoreWhiteSpace=*/true)) {
-  if (Tok.getKind() == tok::raw_identifier &&
-  Tok.getRawIdentifier() == "TEMP_FAILURE_RETRY")
-return true;
-}
+ComparisonInTempFailureRetryCheck::ComparisonInTempFailureRetryCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  RawRetryList(Options.get("RetryMacros", "TEMP_FAILURE_RETRY")) {
+  StringRef(RawRetryList).split(RetryMacros, ",", -1, false);
+}
 
-LocStart = Invocation;
-  }
-  return false;
+void ComparisonInTempFailureRetryCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "RetryMacros", RawRetryList);
 }
-} // namespace
 
 void ComparisonInTempFailureRetryCheck::registerMatchers(MatchFinder *Finder) {
   // Both glibc's and Bionic's TEMP_FAILURE_RETRY macros structurally look 
like:
@@ -63,15 +48,43 @@ void 
ComparisonInTempFailureRetryCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
   binaryOperator(hasOperatorName("="),
  hasRHS(ignoringParenCasts(
- 
binaryOperator(isComparisonOperator()).bind("binop"))),
- isRHSATempFailureRetryArg()),
+ 
binaryOperator(isComparisonOperator()).bind("inner"
+  .bind("outer"),
   this);
 }
 
 void ComparisonInTempFailureRetryCheck::check(
 const MatchFinder::MatchResult ) {
-  const auto  = *Result.Nodes.getNodeAs("binop");
-  diag(BinOp.getOperatorLoc(), "top-level comparison in TEMP_FAILURE_RETRY");
+  StringRef RetryMacroName;
+  const auto  = *Result.Nodes.getNodeAs("outer");
+  if (!Node.getBeginLoc().isMacroID())
+return;
+
+  const SourceManager  = *Result.SourceManager;
+  if 
(!SM.isMacroArgExpansion(Node.getRHS()->IgnoreParenCasts()->getBeginLoc()))
+return;
+
+  const LangOptions  = Result.Context->getLangOpts();
+  SourceLocation LocStart = Node.getBeginLoc();
+  while (LocStart.isMacroID()) {
+SourceLocation Invocation = SM.getImmediateMacroCallerLoc(LocStart);
+Token Tok;
+if (!Lexer::getRawToken(SM.getSpellingLoc(Invocation), Tok, SM, Opts,
+/*IgnoreWhiteSpace=*/true)) {
+  if (Tok.getKind() == tok::raw_identifier &&
+  llvm::is_contained(RetryMacros, Tok.getRawIdentifier())) {
+RetryMacroName = Tok.getRawIdentifier();
+break;
+  }
+}
+
+LocStart = Invocation;
+  }
+  if (RetryMacroName.empty())
+return;
+
+  const auto  = *Result.Nodes.getNodeAs("inner");
+  diag(Inner.getOperatorLoc(), "top-level comparison in %0") << RetryMacroName;
 
   // FIXME: FixIts would be nice, but potentially nontrivial when nested macros
   // happen, e.g. `TEMP_FAILURE_RETRY(IS_ZERO(foo()))`

diff  --git 

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

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

@aaron.ballman - I completely agree with you about the testing.  The interfaces 
are tested via 
https://github.com/llvm/llvm-project/blob/master/clang/unittests/DirectoryWatcher/CMakeLists.txt,
 which now that I look at again, seems to need an additional case for system 
name.


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] D88566: be more specific when testing for no fuse-ld warnings

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/Driver/fuse-ld.c:15
 // RUN:   FileCheck %s --check-prefix=CHECK-NO-WARN
-// CHECK-NO-WARN-NOT: warning:
+// CHECK-NO-WARN-NOT: warning: 'fuse-ld'
 

How does this line trigger unrelated warnings? Can you dump it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88566

___
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 inline comments.



Comment at: clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp:87
+DWORD dwLength = GetFinalPathNameByHandleW(hDirectory, NULL, 0, 0);
+std::unique_ptr buffer{new WCHAR[dwLength + 1]};
+(void)GetFinalPathNameByHandleW(hDirectory, buffer.get(), dwLength + 1, 0);

aaron.ballman wrote:
> Is a smart pointer required here or could you use `std::vector` and 
> reserve the space that way?
Sure, I can convert this to a `std::vector` instead.


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 inline comments.



Comment at: clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp:95
+  CreateEvent(NULL, /*bManualReset=*/TRUE, /*bInitialState=*/FALSE, NULL);
+  assert(ovlIO.hEvent);
+

plotfi wrote:
> Can this be an assert with some message or some explanation of the hEvent 
> return value? What happens if hEvent is non-zero on Release builds? 
I debated this myself which is why I added the `assert`.  Honestly, if this 
fails, there is very little that can be done.  This is creating an anonymous 
(unnamed) event.  The failure here would be caused by out-of-memory conditions 
(you're dead anyways) or system is completely out of resources (you're dead 
anyways).  I don't know of any recovery in that situation.  I really would 
prefer to avoid the two-phase construction which is going to be required if we 
want to handle that error scenario.  The event only makes sense after we have 
the directory handle, so I suppose that we could setup the event prior to the 
construction of the watcher itself, but that is just as much two-phase 
construction as adding an `initialize` method.


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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I wonder if we should unit test this functionality by having some tests that 
create and remove files that are watched. I'm not 100% convinced that is a 
great idea, but not having test coverage for the change is also not really a 
great idea either. Thoughts welcome.




Comment at: clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp:22
+
+#include 
 

You should include `llvm/Support/Windows/WindowsSupport.h` not `Windows.h` 
directly.



Comment at: clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp:71
 public:
-  ~DirectoryWatcherWindows() override { }
-  void InitialScan() { }
-  void EventReceivingLoop() { }
-  void StopWork() { }
+  DirectoryWatcherWindows(HANDLE hDirectory, bool WaitForInitialSync,
+  DirectoryWatcherCallback Receiver);

`hDirectory` -> `Directory`



Comment at: clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp:80
+DirectoryWatcherWindows::DirectoryWatcherWindows(
+HANDLE hDirectory, bool WaitForInitialSync,
+DirectoryWatcherCallback Receiver)

`hDirectory` -> `Directory`



Comment at: clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp:86
+  {
+DWORD dwLength = GetFinalPathNameByHandleW(hDirectory, NULL, 0, 0);
+std::unique_ptr buffer{new WCHAR[dwLength + 1]};

You should strip the Hungarian notation prefixes and ensure all the identifiers 
meet our usual naming rules, I'll stop bringing them up.



Comment at: clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp:87
+DWORD dwLength = GetFinalPathNameByHandleW(hDirectory, NULL, 0, 0);
+std::unique_ptr buffer{new WCHAR[dwLength + 1]};
+(void)GetFinalPathNameByHandleW(hDirectory, buffer.get(), dwLength + 1, 0);

Is a smart pointer required here or could you use `std::vector` and 
reserve the space that way?



Comment at: clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp:101
+  // subdirectories, might as well as ...
+  const BOOL bWatchSubtree = TRUE;
+  const DWORD dwNotifyFilter = FILE_NOTIFY_CHANGE_FILE_NAME

You can drop the top-level `const` on value types.



Comment at: clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp:170
+InitialScan();
+  HandlerThread = std::thread([this, WaitForInitialSync]() {
+// If we did not wait for the initial sync, then we should perform the

A newline above this line would be helpful for visual distinction.



Comment at: clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp:223
+
+  // FIXME(compnerd) should we assert that the path is a directory?
+  SmallVector WidePath;

I think we should assert that -- calling this on a file isn't likely to behave 
in a good way.



Comment at: clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp:229
+
+  const DWORD dwDesiredAccess = FILE_LIST_DIRECTORY;
+  const DWORD dwShareMode =

More top-level `const`s


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] D88668: [CUDA] Add support for 11.1

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

Generally speaking the default should be conservative. It does us no good if we 
generate PTX 99.99, but discover that ptxas does not support it. Granted, these 
days 7.0 is also the wrong default as it's pretty ancient. IMO bumping it to 
9.0 and GPU arch to sm_30 would be sensible.

Now, as for the missing version file, we should probably allow explicitly 
specifying the version. We already do it for HIP.




Comment at: clang/lib/Driver/ToolChains/Cuda.cpp:662
+  case CudaVersion::CUDA_111:
+PtxFeature = "+ptx71";
+break;

I do not think LLVM has this feature implemented. I'm not sure how it reacts to 
clang specifying non-existing feature.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88668

___
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

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

Overall looks good.




Comment at: clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp:95
+  CreateEvent(NULL, /*bManualReset=*/TRUE, /*bInitialState=*/FALSE, NULL);
+  assert(ovlIO.hEvent);
+

Can this be an assert with some message or some explanation of the hEvent 
return value? What happens if hEvent is non-zero on Release builds? 


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] D88590: [clangd] Add bencmark for measuring latency of DecisionForest model.

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz added a comment.

Also typo in commit description: bencmark


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88590

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

[PATCH] D88668: [CUDA] Add support for 11.1

[kiwixz via Phabricator via cfe-commits]

kiwixz added a comment.

In D88668#2306393 , @jlebar wrote:

>> It looks like 11.1 doesn't have a version.txt file
>
> Yikes, this is a problem if we can't tell the difference between CUDA 
> versions!

It looks like a blunder from NVIDIA, CUDA 11.1 actually miss most of the 
documentation (pdf files etc).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88668

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

[PATCH] D88659: [FE]Split SuitableAlign into two parts

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Hm, to start with, the current state of this confuses me.

In GCC, the preprocessor macro `__BIGGEST_ALIGNMENT__` was supposed to expose 
the alignment used by `__attribute__((aligned))` (no arg specified), as well 
the alignment used for alloca. However, this is no longer the case on x86: 
`BIGGEST_ALIGNMENT` is 512bits with avx-512 enabled, 256bits with avx enabled, 
and otherwise 128bits. Alloca follows this too. But, `__attribute__((aligned))` 
was fixed at 128bit alignment, regardless of AVX being enabled, in order to not 
break ABI compatibility with structs using that. On other architectures, the 3 
values seem to be always the same.

In clang, we similarly have (before this patch) both 
DefaultAlignForAttributeAligned (used for ``attribute((aligned))`), and 
SuitableAlign (used for the predefined `__BIGGEST_ALIGNMENT__` and alignment 
for alloca). But these values are different on very many architectures...which 
I think is probably wrong. Furthermore, SuitableAlign isn't being adjusted to 
be suitable for vectors, like it is in gcc, which _also_ seems wrong. Looks 
like there's actually an earlier patch to fix that which was never merged: 
https://reviews.llvm.org/D39313

So, anyways -- back to this patch: On AIX PPC, you want alloca to align to 
128bits, `__attribute__((aligned))` to align to 128bits (aka 8 bytes), but 
`__BIGGEST_ALIGNMENT__` to only be 4?

That seems pretty weird, and probably wrong?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88659

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

[PATCH] D88668: [CUDA] Add support for 11.1

[Justin Lebar via Phabricator via cfe-commits]

jlebar added a comment.

> It looks like 11.1 doesn't have a version.txt file

Yikes, this is a problem if we can't tell the difference between CUDA versions!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88668

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

[PATCH] D88668: [CUDA] Add support for 11.1

[kiwixz via Phabricator via cfe-commits]

kiwixz created this revision.
kiwixz added reviewers: jlebar, tra.
kiwixz added a project: clang.
Herald added subscribers: cfe-commits, yaxunl, jholewinski.
kiwixz requested review of this revision.

It looks like 11.1 doesn't have a version.txt file, so I changed the default 
guess in this case from CUDA 7.0 to 11.1.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88668

Files:
  clang/include/clang/Basic/Cuda.h
  clang/lib/Basic/Cuda.cpp
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/Driver/ToolChains/Cuda.cpp

Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -155,9 +155,9 @@
 llvm::ErrorOr> VersionFile =
 FS.getBufferForFile(InstallPath + "/version.txt");
 if (!VersionFile) {
-  // CUDA 7.0 doesn't have a version.txt, so guess that's our version if
-  // version.txt isn't present.
-  Version = CudaVersion::CUDA_70;
+  // CUDA 7.0 and 11.1 don't have a version.txt, so try to guess
+  // if version.txt isn't present.
+  Version = CudaVersion::CUDA_111;
 } else {
   ParseCudaVersionFile((*VersionFile)->getBuffer());
 }
@@ -658,6 +658,9 @@
   // back-end.
   const char *PtxFeature = nullptr;
   switch (CudaInstallation.version()) {
+  case CudaVersion::CUDA_111:
+PtxFeature = "+ptx71";
+break;
   case CudaVersion::CUDA_110:
 PtxFeature = "+ptx70";
 break;
Index: clang/lib/Basic/Targets/NVPTX.cpp
===
--- clang/lib/Basic/Targets/NVPTX.cpp
+++ clang/lib/Basic/Targets/NVPTX.cpp
@@ -45,6 +45,7 @@
 if (!Feature.startswith("+ptx"))
   continue;
 PTXVersion = llvm::StringSwitch(Feature)
+ .Case("+ptx71", 71)
  .Case("+ptx70", 70)
  .Case("+ptx65", 65)
  .Case("+ptx64", 64)
@@ -239,6 +240,8 @@
 return "750";
   case CudaArch::SM_80:
 return "800";
+  case CudaArch::SM_86:
+return "860";
   }
   llvm_unreachable("unhandled CudaArch");
 }();
Index: clang/lib/Basic/Cuda.cpp
===
--- clang/lib/Basic/Cuda.cpp
+++ clang/lib/Basic/Cuda.cpp
@@ -32,6 +32,8 @@
 return "10.2";
   case CudaVersion::CUDA_110:
 return "11.0";
+  case CudaVersion::CUDA_111:
+return "11.1";
   }
   llvm_unreachable("invalid enum");
 }
@@ -48,6 +50,7 @@
   .Case("10.1", CudaVersion::CUDA_101)
   .Case("10.2", CudaVersion::CUDA_102)
   .Case("11.0", CudaVersion::CUDA_110)
+  .Case("11.1", CudaVersion::CUDA_111)
   .Default(CudaVersion::UNKNOWN);
 }
 
@@ -149,6 +152,8 @@
 return CudaVersion::CUDA_100;
   case CudaArch::SM_80:
 return CudaVersion::CUDA_110;
+  case CudaArch::SM_86:
+return CudaVersion::CUDA_111;
   default:
 llvm_unreachable("invalid enum");
   }
@@ -194,6 +199,8 @@
 return CudaVersion::CUDA_102;
   case 110:
 return CudaVersion::CUDA_110;
+  case 111:
+return CudaVersion::CUDA_111;
   default:
 return CudaVersion::UNKNOWN;
   }
Index: clang/include/clang/Basic/Cuda.h
===
--- clang/include/clang/Basic/Cuda.h
+++ clang/include/clang/Basic/Cuda.h
@@ -29,7 +29,8 @@
   CUDA_101,
   CUDA_102,
   CUDA_110,
-  LATEST = CUDA_110,
+  CUDA_111,
+  LATEST = CUDA_111,
   LATEST_SUPPORTED = CUDA_101,
 };
 const char *CudaVersionToString(CudaVersion V);
@@ -54,6 +55,7 @@
   SM_72,
   SM_75,
   SM_80,
+  SM_86,
   GFX600,
   GFX601,
   GFX700,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 45698ac - [clangd] Split DecisionForest Evaluate() into one func per tree.

[Utkarsh Saxena via cfe-commits]

Author: Utkarsh Saxena
Date: 2020-10-01T18:07:23+02:00
New Revision: 45698ac0052ae5b1c5beb739636396a5b7263966

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

LOG: [clangd] Split DecisionForest Evaluate() into one func per tree.

This allows us MSAN to instrument this function. Previous version is not
instrumentable due to it shear volume.

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

Added: 


Modified: 
clang-tools-extra/clangd/quality/CompletionModelCodegen.py

Removed: 




diff  --git a/clang-tools-extra/clangd/quality/CompletionModelCodegen.py 
b/clang-tools-extra/clangd/quality/CompletionModelCodegen.py
index 423e5d14cf52..a1f0cb78037a 100644
--- a/clang-tools-extra/clangd/quality/CompletionModelCodegen.py
+++ b/clang-tools-extra/clangd/quality/CompletionModelCodegen.py
@@ -1,7 +1,7 @@
 """Code generator for Code Completion Model Inference.
 
 Tool runs on the Decision Forest model defined in {model} directory.
-It generates two files: {output_dir}/{filename}.h and 
{output_dir}/{filename}.cpp 
+It generates two files: {output_dir}/{filename}.h and 
{output_dir}/{filename}.cpp
 The generated files defines the Example class named {cpp_class} having all the 
features as class members.
 The generated runtime provides an `Evaluate` function which can be used to 
score a code completion candidate.
 """
@@ -39,34 +39,32 @@ def header_guard(filename):
 
 
 def boost_node(n, label, next_label):
-"""Returns code snippet for a leaf/boost node.
-Adds value of leaf to the score and jumps to the root of the next tree."""
-return "%s: Score += %s; goto %s;" % (
-label, n['score'], next_label)
+"""Returns code snippet for a leaf/boost node."""
+return "%s: return %s;" % (label, n['score'])
 
 
 def if_greater_node(n, label, next_label):
 """Returns code snippet for a if_greater node.
-Jumps to true_label if the Example feature (NUMBER) is greater than the 
threshold. 
-Comparing integers is much faster than comparing floats. Assuming floating 
points 
+Jumps to true_label if the Example feature (NUMBER) is greater than the 
threshold.
+Comparing integers is much faster than comparing floats. Assuming floating 
points
 are represented as IEEE 754, it order-encodes the floats to integers 
before comparing them.
 Control falls through if condition is evaluated to false."""
 threshold = n["threshold"]
-return "%s: if (E.%s >= %s /*%s*/) goto %s;" % (
-label, n['feature'], order_encode(threshold), threshold, 
next_label)
+return "%s: if (E.get%s() >= %s /*%s*/) goto %s;" % (
+label, n['feature'], order_encode(threshold), threshold, next_label)
 
 
 def if_member_node(n, label, next_label):
 """Returns code snippet for a if_member node.
-Jumps to true_label if the Example feature (ENUM) is present in the set of 
enum values 
+Jumps to true_label if the Example feature (ENUM) is present in the set of 
enum values
 described in the node.
 Control falls through if condition is evaluated to false."""
 members = '|'.join([
 "BIT(%s_type::%s)" % (n['feature'], member)
 for member in n["set"]
 ])
-return "%s: if (E.%s & (%s)) goto %s;" % (
-label, n['feature'], members, next_label)
+return "%s: if (E.get%s() & (%s)) goto %s;" % (
+label, n['feature'], members, next_label)
 
 
 def node(n, label, next_label):
@@ -94,8 +92,6 @@ def tree(t, tree_num, node_num):
 """
 label = "t%d_n%d" % (tree_num, node_num)
 code = []
-if node_num == 0:
-code.append("t%d:" % tree_num)
 
 if t["operation"] == "boost":
 code.append(node(t, label=label, next_label="t%d" % (tree_num + 1)))
@@ -119,13 +115,15 @@ def gen_header_code(features_json, cpp_class, filename):
 """Returns code for header declaring the inference runtime.
 
 Declares the Example class named {cpp_class} inside relevant namespaces.
-The Example class contains all the features as class members. This 
+The Example class contains all the features as class members. This
 class can be used to represent a code completion candidate.
 Provides `float Evaluate()` function which can be used to score the 
Example.
 """
 setters = []
+getters = []
 for f in features_json:
 feature = f["name"]
+
 if f["kind"] == "NUMBER":
 # Floats are order-encoded to integers for faster comparison.
 setters.append(
@@ -138,8 +136,15 @@ class can be used to represent a code completion candidate.
 raise ValueError("Unhandled feature type.", f["kind"])
 
 # Class members represent all the features of the Example.
-class_members = ["uint32_t %s = 0;" % f['name'] for f in 

[PATCH] D88536: [clangd] Split DecisionForest Evaluate() into one func per tree.

[Utkarsh Saxena via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG45698ac0052a: [clangd] Split DecisionForest Evaluate() into 
one func per tree. (authored by usaxena95).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88536

Files:
  clang-tools-extra/clangd/quality/CompletionModelCodegen.py

Index: clang-tools-extra/clangd/quality/CompletionModelCodegen.py
===
--- clang-tools-extra/clangd/quality/CompletionModelCodegen.py
+++ clang-tools-extra/clangd/quality/CompletionModelCodegen.py
@@ -1,7 +1,7 @@
 """Code generator for Code Completion Model Inference.
 
 Tool runs on the Decision Forest model defined in {model} directory.
-It generates two files: {output_dir}/{filename}.h and {output_dir}/{filename}.cpp 
+It generates two files: {output_dir}/{filename}.h and {output_dir}/{filename}.cpp
 The generated files defines the Example class named {cpp_class} having all the features as class members.
 The generated runtime provides an `Evaluate` function which can be used to score a code completion candidate.
 """
@@ -39,34 +39,32 @@
 
 
 def boost_node(n, label, next_label):
-"""Returns code snippet for a leaf/boost node.
-Adds value of leaf to the score and jumps to the root of the next tree."""
-return "%s: Score += %s; goto %s;" % (
-label, n['score'], next_label)
+"""Returns code snippet for a leaf/boost node."""
+return "%s: return %s;" % (label, n['score'])
 
 
 def if_greater_node(n, label, next_label):
 """Returns code snippet for a if_greater node.
-Jumps to true_label if the Example feature (NUMBER) is greater than the threshold. 
-Comparing integers is much faster than comparing floats. Assuming floating points 
+Jumps to true_label if the Example feature (NUMBER) is greater than the threshold.
+Comparing integers is much faster than comparing floats. Assuming floating points
 are represented as IEEE 754, it order-encodes the floats to integers before comparing them.
 Control falls through if condition is evaluated to false."""
 threshold = n["threshold"]
-return "%s: if (E.%s >= %s /*%s*/) goto %s;" % (
-label, n['feature'], order_encode(threshold), threshold, next_label)
+return "%s: if (E.get%s() >= %s /*%s*/) goto %s;" % (
+label, n['feature'], order_encode(threshold), threshold, next_label)
 
 
 def if_member_node(n, label, next_label):
 """Returns code snippet for a if_member node.
-Jumps to true_label if the Example feature (ENUM) is present in the set of enum values 
+Jumps to true_label if the Example feature (ENUM) is present in the set of enum values
 described in the node.
 Control falls through if condition is evaluated to false."""
 members = '|'.join([
 "BIT(%s_type::%s)" % (n['feature'], member)
 for member in n["set"]
 ])
-return "%s: if (E.%s & (%s)) goto %s;" % (
-label, n['feature'], members, next_label)
+return "%s: if (E.get%s() & (%s)) goto %s;" % (
+label, n['feature'], members, next_label)
 
 
 def node(n, label, next_label):
@@ -94,8 +92,6 @@
 """
 label = "t%d_n%d" % (tree_num, node_num)
 code = []
-if node_num == 0:
-code.append("t%d:" % tree_num)
 
 if t["operation"] == "boost":
 code.append(node(t, label=label, next_label="t%d" % (tree_num + 1)))
@@ -119,13 +115,15 @@
 """Returns code for header declaring the inference runtime.
 
 Declares the Example class named {cpp_class} inside relevant namespaces.
-The Example class contains all the features as class members. This 
+The Example class contains all the features as class members. This
 class can be used to represent a code completion candidate.
 Provides `float Evaluate()` function which can be used to score the Example.
 """
 setters = []
+getters = []
 for f in features_json:
 feature = f["name"]
+
 if f["kind"] == "NUMBER":
 # Floats are order-encoded to integers for faster comparison.
 setters.append(
@@ -138,8 +136,15 @@
 raise ValueError("Unhandled feature type.", f["kind"])
 
 # Class members represent all the features of the Example.
-class_members = ["uint32_t %s = 0;" % f['name'] for f in features_json]
-
+class_members = [
+"uint32_t %s = 0;" % f['name']
+for f in features_json
+]
+getters = [
+"LLVM_ATTRIBUTE_ALWAYS_INLINE uint32_t get%s() const { return %s; }"
+% (f['name'], f['name'])
+for f in features_json
+]
 nline = "\n  "
 guard = header_guard(filename)
 return """#ifndef %s
@@ -150,6 +155,10 @@
 %s
 class %s {
 public:
+  // Setters.
+  %s
+
+  // Getters.
   %s
 
 private:
@@ -158,18 +167,16 @@
   // Produces an integer that sorts in the same order as F.
   // 

[PATCH] D88664: [AST] do not error on APFloat invalidOp in default mode

[John McCall via Phabricator via cfe-commits]

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

LGTM.


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

https://reviews.llvm.org/D88664

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

[PATCH] D88590: [clangd] Add bencmark for measuring latency of DecisionForest model.

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz added a comment.

Would it make sense to set random seed to something fixed to make this more 
reproducible?




Comment at: 
clang-tools-extra/clangd/benchmarks/CompletionModel/DecisionForestBenchmark.cpp:2
+//===--- DecisionForestBenchmark.cpp - Benchmark for code completion ranking
+// latency ---*- C++ -*-===//
+//

nit: this is supposed to be one-line comment, I think. 



Comment at: 
clang-tools-extra/clangd/benchmarks/CompletionModel/DecisionForestBenchmark.cpp:22
+namespace {
+std::vector GenerateRandomDataset(int NumExamples) {
+  auto FlipCoin = [&](float Probability) {

There are few lint warnings in this file. Please address them.



Comment at: 
clang-tools-extra/clangd/benchmarks/CompletionModel/DecisionForestBenchmark.cpp:73
+  for (auto _ : State) {
+// State.PauseTiming();
+const std::vector Examples = GenerateRandomDataset(100);

These should not be comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88590

___
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 created this revision.
compnerd added a reviewer: amccarth.
Herald added a project: clang.
compnerd requested review of this revision.

This implements the directory watcher on Windows.  It does the most
naive thing for simplicity.  ReadDirectoryChangesW is used to monitor
the changes.  However, in order to support interruption, we must use
overlapped IO, which allows us to use the blocking, synchronous
mechanism.  We create a thread to post the notification to the consumer
to allow the monitoring to continue.  The two threads communicate via a
locked queue.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88666

Files:
  clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp

Index: clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp
===
--- clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp
+++ clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp
@@ -6,17 +6,11 @@
 //
 //===--===//
 
-// TODO: This is not yet an implementation, but it will make it so Windows
-//   builds don't fail.
-
 #include "DirectoryScanner.h"
 #include "clang/DirectoryWatcher/DirectoryWatcher.h"
 
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/ScopeExit.h"
-#include "llvm/Support/AlignOf.h"
-#include "llvm/Support/Errno.h"
-#include "llvm/Support/Mutex.h"
+#include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/Path.h"
 #include 
 #include 
@@ -24,27 +18,232 @@
 #include 
 #include 
 #include 
-#include 
+
+#include 
 
 namespace {
 
+using DirectoryWatcherCallback =
+std::function, bool)>;
+
 using namespace llvm;
 using namespace clang;
 
 class DirectoryWatcherWindows : public clang::DirectoryWatcher {
+  OVERLAPPED ovlIO;
+
+  alignas(DWORD)
+  CHAR Buffer[4 * (sizeof(FILE_NOTIFY_INFORMATION) + MAX_PATH * sizeof(WCHAR))];
+
+  std::thread WatcherThread;
+  std::thread HandlerThread;
+  std::function, bool)> Callback;
+  SmallString Path;
+
+  class EventQueue {
+std::mutex M;
+std::queue Q;
+std::condition_variable CV;
+
+  public:
+void emplace(DirectoryWatcher::Event::EventKind Kind, StringRef Path) {
+  {
+std::unique_lock L(M);
+Q.emplace(Kind, Path);
+  }
+  CV.notify_one();
+}
+
+DirectoryWatcher::Event pop_front() {
+  std::unique_lock L(M);
+  while (true) {
+if (!Q.empty()) {
+  DirectoryWatcher::Event E = Q.front();
+  Q.pop();
+  return E;
+}
+CV.wait(L, [this]() { return !Q.empty(); });
+  }
+}
+  } Q;
+
 public:
-  ~DirectoryWatcherWindows() override { }
-  void InitialScan() { }
-  void EventReceivingLoop() { }
-  void StopWork() { }
+  DirectoryWatcherWindows(HANDLE hDirectory, bool WaitForInitialSync,
+  DirectoryWatcherCallback Receiver);
+
+  ~DirectoryWatcherWindows() override;
+
+  void InitialScan();
 };
+
+DirectoryWatcherWindows::DirectoryWatcherWindows(
+HANDLE hDirectory, bool WaitForInitialSync,
+DirectoryWatcherCallback Receiver)
+: Callback(Receiver) {
+  // Pre-compute the real location as we will be handing over the directory
+  // handle to the watcher and performing synchronous operations.
+  {
+DWORD dwLength = GetFinalPathNameByHandleW(hDirectory, NULL, 0, 0);
+std::unique_ptr buffer{new WCHAR[dwLength + 1]};
+(void)GetFinalPathNameByHandleW(hDirectory, buffer.get(), dwLength + 1, 0);
+sys::windows::UTF16ToUTF8(buffer.get(), dwLength + 1, Path);
+  }
+
+  memset(, 0, sizeof(ovlIO));
+  ovlIO.hEvent =
+  CreateEvent(NULL, /*bManualReset=*/TRUE, /*bInitialState=*/FALSE, NULL);
+  assert(ovlIO.hEvent);
+
+  WatcherThread = std::thread([this, hDirectory]() {
+while (true) {
+  // We do not guarantee subdirectories, but macOS already provides
+  // subdirectories, might as well as ...
+  const BOOL bWatchSubtree = TRUE;
+  const DWORD dwNotifyFilter = FILE_NOTIFY_CHANGE_FILE_NAME
+ | FILE_NOTIFY_CHANGE_DIR_NAME
+ | FILE_NOTIFY_CHANGE_SIZE
+ | FILE_NOTIFY_CHANGE_LAST_ACCESS
+ | FILE_NOTIFY_CHANGE_LAST_WRITE
+ | FILE_NOTIFY_CHANGE_CREATION;
+
+  DWORD BytesTransferred;
+  if (!ReadDirectoryChangesW(hDirectory, Buffer, sizeof(Buffer),
+ bWatchSubtree, dwNotifyFilter,
+ , , NULL)) {
+Q.emplace(DirectoryWatcher::Event::EventKind::WatcherGotInvalidated,
+  "");
+break;
+  }
+
+  if (!GetOverlappedResult(hDirectory, , , TRUE)) {
+Q.emplace(DirectoryWatcher::Event::EventKind::WatcherGotInvalidated,
+  "");
+break;
+  }
+
+  // There was a buffer underrun on the kernel side.  We may have lost
+ 

[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

The added test causes the below assertion in the baseline (w/o the fix):

  ASTTests: ../../git/llvm-project/clang/lib/AST/ExprConstant.cpp:14543: 
llvm::APSInt clang::Expr::EvaluateKnownConstInt(const clang::ASTContext&, 
llvm::SmallVectorImpl >*) const: Assertion `!isValueDependent() && 
"Expression evaluator can't be called on a dependent expression."' failed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88665

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

[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

[Gabor Marton via Phabricator via cfe-commits]

martong created this revision.
martong added reviewers: balazske, teemperor, shafik, a_sidorin.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a project: clang.
martong requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88665

Files:
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/unittests/AST/StructuralEquivalenceTest.cpp


Index: clang/unittests/AST/StructuralEquivalenceTest.cpp
===
--- clang/unittests/AST/StructuralEquivalenceTest.cpp
+++ clang/unittests/AST/StructuralEquivalenceTest.cpp
@@ -976,6 +976,12 @@
   EXPECT_FALSE(testStructuralMatch(t));
 }
 
+TEST_F(StructuralEquivalenceTemplateTest, DependentFieldDecl) {
+  const char *Code = "template  class foo { int a : sizeof(T); };";
+  auto t = makeNamedDecls(Code, Code, Lang_CXX03);
+  EXPECT_TRUE(testStructuralMatch(t));
+}
+
 TEST_F(StructuralEquivalenceTemplateTest, ExplicitBoolSame) {
   auto Decls = makeNamedDecls(
   "template  struct foo {explicit(b) foo(int);};",
Index: clang/lib/AST/ASTStructuralEquivalence.cpp
===
--- clang/lib/AST/ASTStructuralEquivalence.cpp
+++ clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -1280,6 +1280,10 @@
   }
 
   if (Field1->isBitField()) {
+bool isVD1 = Field1->getBitWidth()->isValueDependent();
+bool isVD2 = Field2->getBitWidth()->isValueDependent();
+if (isVD1 || isVD2)
+  return isVD1 && isVD2;
 // Make sure that the bit-fields are the same length.
 unsigned Bits1 = Field1->getBitWidthValue(Context.FromCtx);
 unsigned Bits2 = Field2->getBitWidthValue(Context.ToCtx);


Index: clang/unittests/AST/StructuralEquivalenceTest.cpp
===
--- clang/unittests/AST/StructuralEquivalenceTest.cpp
+++ clang/unittests/AST/StructuralEquivalenceTest.cpp
@@ -976,6 +976,12 @@
   EXPECT_FALSE(testStructuralMatch(t));
 }
 
+TEST_F(StructuralEquivalenceTemplateTest, DependentFieldDecl) {
+  const char *Code = "template  class foo { int a : sizeof(T); };";
+  auto t = makeNamedDecls(Code, Code, Lang_CXX03);
+  EXPECT_TRUE(testStructuralMatch(t));
+}
+
 TEST_F(StructuralEquivalenceTemplateTest, ExplicitBoolSame) {
   auto Decls = makeNamedDecls(
   "template  struct foo {explicit(b) foo(int);};",
Index: clang/lib/AST/ASTStructuralEquivalence.cpp
===
--- clang/lib/AST/ASTStructuralEquivalence.cpp
+++ clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -1280,6 +1280,10 @@
   }
 
   if (Field1->isBitField()) {
+bool isVD1 = Field1->getBitWidth()->isValueDependent();
+bool isVD2 = Field2->getBitWidth()->isValueDependent();
+if (isVD1 || isVD2)
+  return isVD1 && isVD2;
 // Make sure that the bit-fields are the same length.
 unsigned Bits1 = Field1->getBitWidthValue(Context.FromCtx);
 unsigned Bits2 = Field2->getBitWidthValue(Context.ToCtx);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88664: [AST] do not error on APFloat invalidOp in default mode

[Sanjay Patel via Phabricator via cfe-commits]

spatel created this revision.
spatel added reviewers: rjmccall, efriedma, sepavloff.
Herald added a subscriber: mcrosier.
spatel requested review of this revision.

If FP exceptions are ignored, we should not error out of compilation just 
because APFloat indicated an exception. 
This is required as a preliminary step for D88238 
 which changes APFloat behavior for signaling 
NaN convert() to set the opInvalidOp exception status.

Currently, there is no way to trigger this error because convert() never sets 
opInvalidOp. FP binops that set opInvalidOp also create a NaN, so the path to 
checkFloatingPointResult() is blocked by a different diagnostic:

  // [expr.pre]p4:
  //   If during the evaluation of an expression, the result is not
  //   mathematically defined [...], the behavior is undefined.
  // FIXME: C++ rules require us to not conform to IEEE 754 here.
  if (LHS.isNaN()) {
Info.CCEDiag(E, diag::note_constexpr_float_arithmetic) << LHS.isNaN();
return Info.noteUndefinedBehavior();
  }
  return checkFloatingPointResult(Info, E, St);


https://reviews.llvm.org/D88664

Files:
  clang/lib/AST/ExprConstant.cpp


Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -2439,7 +2439,8 @@
 return false;
   }
 
-  if (St & APFloat::opStatus::opInvalidOp) {
+  if ((St & APFloat::opStatus::opInvalidOp) &&
+  FPO.getFPExceptionMode() != LangOptions::FPE_Ignore) {
 // There is no usefully definable result.
 Info.FFDiag(E);
 return false;


Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -2439,7 +2439,8 @@
 return false;
   }
 
-  if (St & APFloat::opStatus::opInvalidOp) {
+  if ((St & APFloat::opStatus::opInvalidOp) &&
+  FPO.getFPExceptionMode() != LangOptions::FPE_Ignore) {
 // There is no usefully definable result.
 Info.FFDiag(E);
 return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88446: docs: add documentation describing API Notes

[Saleem Abdulrasool via Phabricator via cfe-commits]

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



Comment at: clang/docs/APINotes.rst:45
+Clang will search for API notes files next to module maps only when passed the
+``-fapinotes-modules`` option.
+

rsmith wrote:
> Can we add a hyphen between `api` and `notes` here? `-fapinotes` is a little 
> hard to read.
Sure; we can always add a `-fapinotes-modules` as a silent alias for backwards 
compatibility.  I assume you would like the same for the rest of the options, 
and have made that change through out.



Comment at: clang/docs/APINotes.rst:187-190
+  - "N"onnull (``_Nonnull``)
+  - "O"ptional (``_Nullable``)
+  - "U"nspecified (``_Null_unspecified``)
+  - "S"calar (deprecated)

rsmith wrote:
> Is it important that these are single letters? Spelling out the name in full 
> (as you do for other enumerated values like `MethodKind` and `PropertyKind`) 
> would seem a little more readable. (We could accept the single-letter forms 
> as aliases.)
I don't think that they be single letters is important.  As long as we have the 
compatibility aliases, I think it should be fine to support the fully spelt out 
versions, as the compatibility is needed for existing APINotes.



Comment at: clang/docs/APINotes.rst:233-235
+  Note that the type is *not* parsed in the context where it will be used,
+  which means that macros are not available and nullability must be applied
+  explicitly (even in an ``NS_ASSUME_NONNULL_BEGIN`` section).

rsmith wrote:
> So what context is it parsed in? Below you have an `NSArray *` example; how 
> do we do the lookup for `NSArray`?
A separate buffer is constructed where the annotations are processed.  During 
semantic analysis, the requested APINotes are processed and the attributes 
specified are applied to the declaration.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88446

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

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.
Herald added a subscriber: tatianashp.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

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

[PATCH] D88314: Added llvm-string-referencing check

[Bogdan Serea via Phabricator via cfe-commits]

bogser01 updated this revision to Diff 295580.
bogser01 added a comment.

Fixed unit test 2


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88314

Files:
  clang-tools-extra/clang-tidy/add_new_check.py
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/llvm/StringReferencingCheck.cpp
  clang-tools-extra/clang-tidy/llvm/StringReferencingCheck.h
  clang-tools-extra/docs/clang-tidy/Contributing.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-string-referencing.rst
  clang-tools-extra/test/clang-tidy/checkers/llvm-string-referencing.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/llvm-string-referencing.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvm-string-referencing.cpp
@@ -0,0 +1,66 @@
+// RUN: %check_clang_tidy %s llvm-string-referencing %t
+
+namespace std {
+class string {};
+class u18_string_t;
+
+} // namespace std
+
+namespace llvm {
+class StringRef;
+} // namespace llvm
+
+class String;
+
+namespace A {
+using namespace std;
+void f(const string );
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing]
+// CHECK-FIXES: void f(llvm::StringRef P);{{$}}
+} // namespace A
+
+namespace B {
+using std::string;
+void f1(const string );
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing]
+// CHECK-FIXES: void f1(llvm::StringRef P);{{$}}
+} // namespace B
+
+void f2(std::string, int, const std::string &);
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing]
+// CHECK-FIXES: void f2(std::string, int, llvm::StringRef );{{$}}
+void f2(std::string P, int x, const std::string ) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing]
+  // CHECK-FIXES: void f2(std::string P, int x, llvm::StringRef P2) {{{$}}
+  return;
+}
+
+void f3(const std::string , const std::string );
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing]
+// CHECK-MESSAGES: :[[@LINE-2]]:32: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing]
+// CHECK-FIXES: void f3(llvm::StringRef P1, llvm::StringRef P2);{{$}}
+
+struct St {
+  void operator=(const std::string ) const {
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing]
+// CHECK-FIXES: void operator=(llvm::StringRef Val) const {{{$}}
+return;
+  }
+};
+
+void f7(const std::string &);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing]
+// CHECK-FIXES: void f7(llvm::StringRef );{{$}}
+
+// Functions below this line should not trigger the check
+void f1(std::string );
+
+void f4(std::string *P);
+
+void f5(String );
+
+void f6(llvm::StringRef P);
+
+void f9(std::u18_string_t );
+
+void f10(const std::string &);
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-string-referencing.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvm-string-referencing.rst
@@ -0,0 +1,6 @@
+.. title:: clang-tidy - llvm-string-referencing
+
+llvm-string-referencing
+===
+
+FIXME: Describe what patterns does the check detect and why. Give examples.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -192,6 +192,7 @@
`llvm-namespace-comment `_,
`llvm-prefer-isa-or-dyn-cast-in-conditionals `_, "Yes"
`llvm-prefer-register-over-unsigned `_, "Yes"
+   `llvm-string-referencing `_, "Yes"
`llvm-twine-local `_, "Yes"
`llvmlibc-callee-namespace `_,
`llvmlibc-implementation-in-namespace `_,
Index: clang-tools-extra/docs/clang-tidy/Contributing.rst
===
--- clang-tools-extra/docs/clang-tidy/Contributing.rst
+++ clang-tools-extra/docs/clang-tidy/Contributing.rst
@@ -201,8 +201,13 @@
   }
 
   void AwesomeFunctionNamesCheck::check(const MatchFinder::MatchResult ) {
+<<< HEAD
+const auto *MatchedDecl = Result.Nodes.getNodeAs("x");
+if ((!MatchedDecl->getIdentifier()) || MatchedDecl->getName().startswith("awesome_"))

[clang] 8c36eaf - [clang][opencl][codegen] Remove the insertion of `correctly-rounded-divide-sqrt-fp-math` fn-attr.

[Michael Liao via cfe-commits]

Author: Michael Liao
Date: 2020-10-01T11:07:39-04:00
New Revision: 8c36eaf0377285acb89c319582d9666e60f42007

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

LOG: [clang][opencl][codegen] Remove the insertion of 
`correctly-rounded-divide-sqrt-fp-math` fn-attr.

- `-cl-fp32-correctly-rounded-divide-sqrt` is already handled in a
  per-instruction manner by annotating the accuracy required. There's no
  need to add that fn-attr. So far, there's no in-tree backend handling
  that attr and that OpenCL specific option.
- In case that out-of-tree backends are broken, this change could be
  reverted if those backends could not be fixed.

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

Added: 


Modified: 
clang/lib/CodeGen/CGCall.cpp
clang/test/CodeGenOpenCL/amdgpu-attrs.cl
clang/test/CodeGenOpenCL/fpmath.cl

Removed: 




diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index ec7ddf8b5d9e..cb03e025e19e 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -1794,11 +1794,6 @@ void 
CodeGenModule::getDefaultFunctionAttributes(StringRef Name,
llvm::utostr(CodeGenOpts.SSPBufferSize));
 FuncAttrs.addAttribute("no-signed-zeros-fp-math",
llvm::toStringRef(LangOpts.NoSignedZero));
-if (getLangOpts().OpenCL) {
-  FuncAttrs.addAttribute(
-  "correctly-rounded-divide-sqrt-fp-math",
-  llvm::toStringRef(CodeGenOpts.CorrectlyRoundedDivSqrt));
-}
 
 // TODO: Reciprocal estimate codegen options should apply to instructions?
 const std::vector  = CodeGenOpts.Reciprocals;

diff  --git a/clang/test/CodeGenOpenCL/amdgpu-attrs.cl 
b/clang/test/CodeGenOpenCL/amdgpu-attrs.cl
index 13f8b1191c2b..9156c45f4939 100644
--- a/clang/test/CodeGenOpenCL/amdgpu-attrs.cl
+++ b/clang/test/CodeGenOpenCL/amdgpu-attrs.cl
@@ -190,5 +190,5 @@ kernel void default_kernel() {
 // CHECK-DAG: attributes 
[[FLAT_WORK_GROUP_SIZE_32_64_WAVES_PER_EU_2_NUM_SGPR_32_NUM_VGPR_64]] = {{.*}} 
"amdgpu-flat-work-group-size"="32,64" "amdgpu-implicitarg-num-bytes"="56" 
"amdgpu-num-sgpr"="32" "amdgpu-num-vgpr"="64" "amdgpu-waves-per-eu"="2"
 // CHECK-DAG: attributes 
[[FLAT_WORK_GROUP_SIZE_32_64_WAVES_PER_EU_2_4_NUM_SGPR_32_NUM_VGPR_64]] = 
{{.*}} "amdgpu-flat-work-group-size"="32,64" 
"amdgpu-implicitarg-num-bytes"="56" "amdgpu-num-sgpr"="32" 
"amdgpu-num-vgpr"="64" "amdgpu-waves-per-eu"="2,4"
 
-// CHECK-DAG: attributes [[A_FUNCTION]] = {{.*}} 
"correctly-rounded-divide-sqrt-fp-math"="false"
+// CHECK-DAG: attributes [[A_FUNCTION]] = {{.*}}
 // CHECK-DAG: attributes [[DEFAULT_KERNEL_ATTRS]] = {{.*}} 
"amdgpu-flat-work-group-size"="1,256" "amdgpu-implicitarg-num-bytes"="56"

diff  --git a/clang/test/CodeGenOpenCL/fpmath.cl 
b/clang/test/CodeGenOpenCL/fpmath.cl
index 0108d909c94e..36cb8e68ea7c 100644
--- a/clang/test/CodeGenOpenCL/fpmath.cl
+++ b/clang/test/CodeGenOpenCL/fpmath.cl
@@ -7,7 +7,6 @@ typedef __attribute__(( ext_vector_type(4) )) float float4;
 
 float spscalardiv(float a, float b) {
   // CHECK: @spscalardiv
-  // CHECK: #[[ATTR:[0-9]+]]
   // CHECK: fdiv{{.*}},
   // NODIVOPT: !fpmath ![[MD:[0-9]+]]
   // DIVOPT-NOT: !fpmath ![[MD:[0-9]+]]
@@ -16,7 +15,6 @@ float spscalardiv(float a, float b) {
 
 float4 spvectordiv(float4 a, float4 b) {
   // CHECK: @spvectordiv
-  // CHECK: #[[ATTR2:[0-9]+]]
   // CHECK: fdiv{{.*}},
   // NODIVOPT: !fpmath ![[MD]]
   // DIVOPT-NOT: !fpmath ![[MD]]
@@ -38,18 +36,9 @@ void testdbllit(long *val) {
 #pragma OPENCL EXTENSION cl_khr_fp64 : enable
 double dpscalardiv(double a, double b) {
   // CHECK: @dpscalardiv
-  // CHECK: #[[ATTR]]
   // CHECK-NOT: !fpmath
   return a / b;
 }
 #endif
 
-// CHECK: attributes #[[ATTR]] = {
-// NODIVOPT-SAME: "correctly-rounded-divide-sqrt-fp-math"="false"
-// DIVOPT-SAME: "correctly-rounded-divide-sqrt-fp-math"="true"
-// CHECK-SAME: }
-// CHECK: attributes #[[ATTR2]] = {
-// NODIVOPT-SAME: "correctly-rounded-divide-sqrt-fp-math"="false"
-// DIVOPT-SAME: "correctly-rounded-divide-sqrt-fp-math"="true"
-// CHECK-SAME: }
 // NODIVOPT: ![[MD]] = !{float 2.50e+00}



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

[PATCH] D88424: [clang][codegen] Remove the insertion of `correctly-rounded-divide-sqrt-fp-math` fn-attr.

[Michael Liao via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8c36eaf03772: [clang][opencl][codegen] Remove the insertion 
of `correctly-rounded-divide-sqrt… (authored by hliao).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88424

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGenOpenCL/amdgpu-attrs.cl
  clang/test/CodeGenOpenCL/fpmath.cl


Index: clang/test/CodeGenOpenCL/fpmath.cl
===
--- clang/test/CodeGenOpenCL/fpmath.cl
+++ clang/test/CodeGenOpenCL/fpmath.cl
@@ -7,7 +7,6 @@
 
 float spscalardiv(float a, float b) {
   // CHECK: @spscalardiv
-  // CHECK: #[[ATTR:[0-9]+]]
   // CHECK: fdiv{{.*}},
   // NODIVOPT: !fpmath ![[MD:[0-9]+]]
   // DIVOPT-NOT: !fpmath ![[MD:[0-9]+]]
@@ -16,7 +15,6 @@
 
 float4 spvectordiv(float4 a, float4 b) {
   // CHECK: @spvectordiv
-  // CHECK: #[[ATTR2:[0-9]+]]
   // CHECK: fdiv{{.*}},
   // NODIVOPT: !fpmath ![[MD]]
   // DIVOPT-NOT: !fpmath ![[MD]]
@@ -38,18 +36,9 @@
 #pragma OPENCL EXTENSION cl_khr_fp64 : enable
 double dpscalardiv(double a, double b) {
   // CHECK: @dpscalardiv
-  // CHECK: #[[ATTR]]
   // CHECK-NOT: !fpmath
   return a / b;
 }
 #endif
 
-// CHECK: attributes #[[ATTR]] = {
-// NODIVOPT-SAME: "correctly-rounded-divide-sqrt-fp-math"="false"
-// DIVOPT-SAME: "correctly-rounded-divide-sqrt-fp-math"="true"
-// CHECK-SAME: }
-// CHECK: attributes #[[ATTR2]] = {
-// NODIVOPT-SAME: "correctly-rounded-divide-sqrt-fp-math"="false"
-// DIVOPT-SAME: "correctly-rounded-divide-sqrt-fp-math"="true"
-// CHECK-SAME: }
 // NODIVOPT: ![[MD]] = !{float 2.50e+00}
Index: clang/test/CodeGenOpenCL/amdgpu-attrs.cl
===
--- clang/test/CodeGenOpenCL/amdgpu-attrs.cl
+++ clang/test/CodeGenOpenCL/amdgpu-attrs.cl
@@ -190,5 +190,5 @@
 // CHECK-DAG: attributes 
[[FLAT_WORK_GROUP_SIZE_32_64_WAVES_PER_EU_2_NUM_SGPR_32_NUM_VGPR_64]] = {{.*}} 
"amdgpu-flat-work-group-size"="32,64" "amdgpu-implicitarg-num-bytes"="56" 
"amdgpu-num-sgpr"="32" "amdgpu-num-vgpr"="64" "amdgpu-waves-per-eu"="2"
 // CHECK-DAG: attributes 
[[FLAT_WORK_GROUP_SIZE_32_64_WAVES_PER_EU_2_4_NUM_SGPR_32_NUM_VGPR_64]] = 
{{.*}} "amdgpu-flat-work-group-size"="32,64" 
"amdgpu-implicitarg-num-bytes"="56" "amdgpu-num-sgpr"="32" 
"amdgpu-num-vgpr"="64" "amdgpu-waves-per-eu"="2,4"
 
-// CHECK-DAG: attributes [[A_FUNCTION]] = {{.*}} 
"correctly-rounded-divide-sqrt-fp-math"="false"
+// CHECK-DAG: attributes [[A_FUNCTION]] = {{.*}}
 // CHECK-DAG: attributes [[DEFAULT_KERNEL_ATTRS]] = {{.*}} 
"amdgpu-flat-work-group-size"="1,256" "amdgpu-implicitarg-num-bytes"="56"
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1794,11 +1794,6 @@
llvm::utostr(CodeGenOpts.SSPBufferSize));
 FuncAttrs.addAttribute("no-signed-zeros-fp-math",
llvm::toStringRef(LangOpts.NoSignedZero));
-if (getLangOpts().OpenCL) {
-  FuncAttrs.addAttribute(
-  "correctly-rounded-divide-sqrt-fp-math",
-  llvm::toStringRef(CodeGenOpts.CorrectlyRoundedDivSqrt));
-}
 
 // TODO: Reciprocal estimate codegen options should apply to instructions?
 const std::vector  = CodeGenOpts.Reciprocals;


Index: clang/test/CodeGenOpenCL/fpmath.cl
===
--- clang/test/CodeGenOpenCL/fpmath.cl
+++ clang/test/CodeGenOpenCL/fpmath.cl
@@ -7,7 +7,6 @@
 
 float spscalardiv(float a, float b) {
   // CHECK: @spscalardiv
-  // CHECK: #[[ATTR:[0-9]+]]
   // CHECK: fdiv{{.*}},
   // NODIVOPT: !fpmath ![[MD:[0-9]+]]
   // DIVOPT-NOT: !fpmath ![[MD:[0-9]+]]
@@ -16,7 +15,6 @@
 
 float4 spvectordiv(float4 a, float4 b) {
   // CHECK: @spvectordiv
-  // CHECK: #[[ATTR2:[0-9]+]]
   // CHECK: fdiv{{.*}},
   // NODIVOPT: !fpmath ![[MD]]
   // DIVOPT-NOT: !fpmath ![[MD]]
@@ -38,18 +36,9 @@
 #pragma OPENCL EXTENSION cl_khr_fp64 : enable
 double dpscalardiv(double a, double b) {
   // CHECK: @dpscalardiv
-  // CHECK: #[[ATTR]]
   // CHECK-NOT: !fpmath
   return a / b;
 }
 #endif
 
-// CHECK: attributes #[[ATTR]] = {
-// NODIVOPT-SAME: "correctly-rounded-divide-sqrt-fp-math"="false"
-// DIVOPT-SAME: "correctly-rounded-divide-sqrt-fp-math"="true"
-// CHECK-SAME: }
-// CHECK: attributes #[[ATTR2]] = {
-// NODIVOPT-SAME: "correctly-rounded-divide-sqrt-fp-math"="false"
-// DIVOPT-SAME: "correctly-rounded-divide-sqrt-fp-math"="true"
-// CHECK-SAME: }
 // NODIVOPT: ![[MD]] = !{float 2.50e+00}
Index: clang/test/CodeGenOpenCL/amdgpu-attrs.cl
===
--- clang/test/CodeGenOpenCL/amdgpu-attrs.cl
+++ clang/test/CodeGenOpenCL/amdgpu-attrs.cl
@@ -190,5 +190,5 @@
 // CHECK-DAG: attributes 

[PATCH] D88659: [FE]Split SuitableAlign into two parts

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L created this revision.
Xiangling_L added reviewers: hubert.reinterpretcast, zarko, Jason, jyknight, 
efriedma.
Herald added subscribers: cfe-commits, luismarques, apazos, sameer.abuasal, 
pzheng, s.egerton, lenary, Jim, jocewei, PkmX, the_o, brucehoult, 
MartinMosbeck, rogfer01, atanasyan, edward-jones, zzheng, jrtc27, niosHD, 
sabuasal, simoncook, johnrusso, rbar, asb, fedor.sergeev, kbarton, 
jgravelle-google, sbc100, nemanjai, sdardis, dylanmckay, dschuff.
Herald added a project: clang.
Xiangling_L requested review of this revision.
Herald added subscribers: MaskRay, aheejin.

There are only two places where "SuitableAlign" is used:

- calculate 'BIGGEST_ALIGNMENT' macro
- alignment for 'Alloca' Inst

On some targets, like AIX, the two value are not equal. So we split 
`SuitableAlign` into two parts to meet the needs.

One is 'GuanranteedAlign'[?any better name] which is used to calculate 
'BIGGEST_ALIGNMENT' and represent fundamental alignment requirement; the other 
one is still 'SuitableAlign' used to calculate alignment for 'Alloca' Inst.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88659

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/ARC.h
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Basic/Targets/AVR.h
  clang/lib/Basic/Targets/MSP430.h
  clang/lib/Basic/Targets/Mips.h
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Basic/Targets/Sparc.h
  clang/lib/Basic/Targets/TCE.h
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/Basic/Targets/XCore.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/CodeGen/aix_alloca_align.c

Index: clang/test/CodeGen/aix_alloca_align.c
===
--- /dev/null
+++ clang/test/CodeGen/aix_alloca_align.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple=powerpc-ibm-aix-xcoff -S -emit-llvm < %s | \
+// RUN: FileCheck -check-prefix 32BIT %s
+
+// RUN: %clang_cc1 -triple=powerpc64-ibm-aix-xcoff -S -emit-llvm < %s | \
+// RUN: FileCheck -check-prefix 64BIT %s
+
+typedef long unsigned int size_t;
+extern void *alloca(size_t __size) __attribute__((__nothrow__));
+
+void foo() {
+  char *ptr1 = (char *)alloca(sizeof(char) * 9);
+  int *ptr2 = (int *)alloca(sizeof(int) * 9);
+  double *ptr3 = (double *)alloca(sizeof(double) * 9);
+}
+
+// 32BIT: %0 = alloca i8, i32 9, align 16
+// 32BIT: %1 = alloca i8, i32 36, align 16
+// 32BIT: %3 = alloca i8, i32 72, align 16
+
+// 64BIT: %0 = alloca i8, i64 9, align 16
+// 64BIT: %1 = alloca i8, i64 36, align 16
+// 64BIT: %3 = alloca i8, i64 72, align 16
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -882,7 +882,7 @@
 
   // Define __BIGGEST_ALIGNMENT__ to be compatible with gcc.
   Builder.defineMacro("__BIGGEST_ALIGNMENT__",
-  Twine(TI.getSuitableAlign() / TI.getCharWidth()) );
+  Twine(TI.getGuaranteedAlign() / TI.getCharWidth()));
 
   if (!LangOpts.CharIsSigned)
 Builder.defineMacro("__CHAR_UNSIGNED__");
Index: clang/lib/Basic/Targets/XCore.h
===
--- clang/lib/Basic/Targets/XCore.h
+++ clang/lib/Basic/Targets/XCore.h
@@ -29,7 +29,7 @@
   : TargetInfo(Triple) {
 NoAsmVariants = true;
 LongLongAlign = 32;
-SuitableAlign = 32;
+GuaranteedAlign = SuitableAlign = 32;
 DoubleAlign = LongDoubleAlign = 32;
 SizeType = UnsignedInt;
 PtrDiffType = SignedInt;
Index: clang/lib/Basic/Targets/X86.h
===
--- clang/lib/Basic/Targets/X86.h
+++ clang/lib/Basic/Targets/X86.h
@@ -381,7 +381,7 @@
 DoubleAlign = LongLongAlign = 32;
 LongDoubleWidth = 96;
 LongDoubleAlign = 32;
-SuitableAlign = 128;
+GuaranteedAlign = SuitableAlign = 128;
 resetDataLayout(Triple.isOSBinFormatMachO() ?
 "e-m:o-p:32:32-p270:32:32-p271:32:32-p272:64:64-f64:32:64-"
 "f80:32-n8:16:32-S128" :
@@ -481,7 +481,7 @@
   : DarwinTargetInfo(Triple, Opts) {
 LongDoubleWidth = 128;
 LongDoubleAlign = 128;
-SuitableAlign = 128;
+GuaranteedAlign = SuitableAlign = 128;
 MaxVectorAlign = 256;
 // The watchOS simulator uses the builtin bool type for Objective-C.
 llvm::Triple T = llvm::Triple(Triple);
@@ -655,7 +655,7 @@
 LongDoubleAlign = 128;
 LargeArrayMinWidth = 128;
 LargeArrayAlign = 128;
-SuitableAlign = 128;
+GuaranteedAlign = SuitableAlign = 128;
 SizeType = IsX32 ? UnsignedInt : UnsignedLong;
 PtrDiffType = IsX32 ? SignedInt : SignedLong;
 IntPtrType = IsX32 ? SignedInt : SignedLong;
@@ -893,7 +893,7 @@
 public:
   AndroidX86_32TargetInfo(const 

[PATCH] D88491: [ASTContext] Use AllowCXX in all merge*Type methods, strip references

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Do you have test cases for this change? I didn't see any relevant ones in 
D88384 .




Comment at: clang/lib/AST/ASTContext.cpp:9174
 bool ASTContext::typesAreBlockPointerCompatible(QualType LHS, QualType RHS) {
   return !mergeTypes(LHS, RHS, true).isNull();
 }

It seems possible to call `typesAreBlockPointerCompatible()` in C++ mode 
(`ASTContext::areCommonBaseCompatible()` calls `sameObjCTypeArgs()` which calls 
`canAssignObjCObjectTypes()` which calls 
`ASTContext::typesAreBlockPointerCompatible()`. I'm not certain if the 
assertion will actually trigger though, so tests would be appreciated.

Are there other cases where `mergeType()` can be transitively called in C++ 
mode without having already stripped references?



Comment at: clang/lib/AST/ASTContext.cpp:9430-9433
+  if (const ReferenceType *lRT = LHS->getAs())
+LHS = lRT->getPointeeType();
+  if (const ReferenceType *rRT = RHS->getAs())
+RHS = rRT->getPointeeType();

This will try to merge a `T&` and `T&&` based on `T` alone, is that expected or 
should this be caring about lvalue vs rvalue reference types?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88491

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

[PATCH] D88524: [CUDA][HIP] Fix bound arch for offload action for fat binary

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 295572.
yaxunl added a comment.
Herald added a subscriber: jholewinski.

add CudaArch::UNUSED as suggested by Artem.


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

https://reviews.llvm.org/D88524

Files:
  clang/include/clang/Basic/Cuda.h
  clang/lib/Basic/Cuda.cpp
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/hip-phases.hip
  clang/test/Driver/hip-toolchain-device-only.hip

Index: clang/test/Driver/hip-toolchain-device-only.hip
===
--- /dev/null
+++ clang/test/Driver/hip-toolchain-device-only.hip
@@ -0,0 +1,29 @@
+// REQUIRES: clang-driver, amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --offload-arch=gfx803 --offload-arch=gfx900 \
+// RUN:   --cuda-device-only -nogpuinc -nogpulib -c \
+// RUN:   %s 2>&1 | FileCheck -check-prefixes=CHECK,LINK %s
+
+// CHECK-NOT: error:
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// CHECK-SAME: "-fcuda-is-device"
+// CHECK-SAME: "-target-cpu" "gfx803"
+// CHECK-SAME: {{.*}} "-o" [[OBJ_DEV_A_803:".*o"]] "-x" "hip"
+
+// CHECK: [[LLD: ".*lld.*"]] "-flavor" "gnu" "--no-undefined" "-shared"
+// CHECK-SAME: "-o" "[[IMG_DEV_A_803:.*out]]" [[OBJ_DEV_A_803]]
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// CHECK-SAME: "-emit-obj"
+// CHECK-SAME: "-fcuda-is-device"
+// CHECK-SAME: "-target-cpu" "gfx900"
+// CHECK-SAME: {{.*}} "-o" [[OBJ_DEV_A_900:".*o"]] "-x" "hip"
+
+// CHECK: [[LLD]] "-flavor" "gnu" "--no-undefined" "-shared"
+// CHECK-SAME: "-o" "[[IMG_DEV_A_900:.*out]]" [[OBJ_DEV_A_900]]
+
+// CHECK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
+// CHECK-SAME: "-targets={{.*}},hip-amdgcn-amd-amdhsa-gfx803,hip-amdgcn-amd-amdhsa-gfx900"
+// CHECK-SAME: "-inputs={{.*}},[[IMG_DEV_A_803]],[[IMG_DEV_A_900]]" "-outputs=[[BUNDLE_A:.*hipfb]]"
Index: clang/test/Driver/hip-phases.hip
===
--- clang/test/Driver/hip-phases.hip
+++ clang/test/Driver/hip-phases.hip
@@ -219,6 +219,12 @@
 // DBIN-DAG: [[P0:[0-9]+]]: input, "{{.*}}hip-phases.hip", [[T:hip]], (device-[[T]], [[ARCH:gfx803]])
 // DBIN-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, [[T]]-cpp-output, (device-[[T]], [[ARCH]])
 // DBIN-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (device-[[T]], [[ARCH]])
+// DBIN-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (device-[[T]], [[ARCH]])
+// DBIN-DAG: [[P4:[0-9]+]]: assembler, {[[P3]]}, object, (device-[[T]], [[ARCH]])
+// DBIN-DAG: [[P5:[0-9]+]]: linker, {[[P4]]}, image, (device-[[T]], [[ARCH]])
+// DBIN-DAG: [[P6:[0-9]+]]: offload, "device-[[T]] (amdgcn-amd-amdhsa:[[ARCH]])" {[[P5]]}, image
+// DBIN-DAG: [[P7:[0-9]+]]: linker, {[[P6]]}, hip-fatbin, (device-hip, )
+// DBIN-DAG: [[P8:[0-9]+]]: offload, "device-[[T]] (amdgcn-amd-amdhsa:)" {[[P7]]}, hip-fatbin
 // DBIN-NOT: host
 //
 // Test single gpu architecture up to the assemble phase in device-only
@@ -230,6 +236,8 @@
 // DASM-DAG: [[P0:[0-9]+]]: input, "{{.*}}hip-phases.hip", [[T:hip]], (device-[[T]], [[ARCH:gfx803]])
 // DASM-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, [[T]]-cpp-output, (device-[[T]], [[ARCH]])
 // DASM-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (device-[[T]], [[ARCH]])
+// DASM-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (device-[[T]], [[ARCH]])
+// DASM-DAG: [[P4:[0-9]+]]: offload, "device-[[T]] (amdgcn-amd-amdhsa:[[ARCH]])" {[[P3]]}, assembler
 // DASM-NOT: host
 
 //
@@ -242,9 +250,19 @@
 // DBIN2-DAG: [[P0:[0-9]+]]: input, "{{.*}}hip-phases.hip", [[T:hip]], (device-[[T]], [[ARCH:gfx803]])
 // DBIN2-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, [[T]]-cpp-output, (device-[[T]], [[ARCH]])
 // DBIN2-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (device-[[T]], [[ARCH]])
-// DBIN2-DAG: [[P6:[0-9]+]]: input, "{{.*}}hip-phases.hip", [[T]], (device-[[T]], [[ARCH2:gfx900]])
-// DBIN2-DAG: [[P7:[0-9]+]]: preprocessor, {[[P6]]}, [[T]]-cpp-output, (device-[[T]], [[ARCH2]])
-// DBIN2-DAG: [[P8:[0-9]+]]: compiler, {[[P7]]}, ir, (device-[[T]], [[ARCH2]])
+// DBIN2-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (device-[[T]], [[ARCH]])
+// DBIN2-DAG: [[P4:[0-9]+]]: assembler, {[[P3]]}, object, (device-[[T]], [[ARCH]])
+// DBIN2-DAG: [[P5:[0-9]+]]: linker, {[[P4]]}, image, (device-[[T]], [[ARCH]])
+// DBIN2-DAG: [[P6:[0-9]+]]: offload, "device-[[T]] (amdgcn-amd-amdhsa:[[ARCH]])" {[[P5]]}, image
+// DBIN2-DAG: [[P7:[0-9]+]]: input, "{{.*}}hip-phases.hip", [[T]], (device-[[T]], [[ARCH2:gfx900]])
+// DBIN2-DAG: [[P8:[0-9]+]]: preprocessor, {[[P7]]}, [[T]]-cpp-output, (device-[[T]], [[ARCH2]])
+// DBIN2-DAG: [[P9:[0-9]+]]: compiler, {[[P8]]}, ir, (device-[[T]], [[ARCH2]])
+// DBIN2-DAG: [[P10:[0-9]+]]: backend, {[[P9]]}, assembler, (device-[[T]], [[ARCH2]])
+// DBIN2-DAG: [[P11:[0-9]+]]: assembler, {[[P10]]}, object, (device-[[T]], [[ARCH2]])
+// DBIN2-DAG: 

[PATCH] D87962: [clang] Change the multi-character character constants from extension to implementation-defined.

[Aaron Ballman via Phabricator via cfe-commits]

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

In D87962#2305375 , @nomanous wrote:

> In D87962#2298021 , @aaron.ballman 
> wrote:
>
>> Multichar literals are implementation-defined in C and conditionally 
>> supported with implementation-defined semantics in C++. I agree that it may 
>> make sense to warn about their use for portability reasons, but I'm not 
>> certain whether it makes sense to promote their use to be always-on 
>> diagnostics. I'm curious to know if this change causes any issues with 
>> system headers (which may or may not still define four char codes) or 
>> popular libraries.
>>
>> I was curious as to why this was an extension in the first place and found 
>> the original commits 
>> (https://github.com/llvm/llvm-project/commit/74c95e20af4838152a63010292d1063835176711
>>  and 
>> https://github.com/llvm/llvm-project/commit/8577f62622d50183c7413d7507ec783d3c1486fc)
>>  but there's no justification as to why this was picked as an extension.
>
> Or should we change the four character case to "Remark" so it wouldn't be 
> warned even with the "-pandemic" option? There seems no other choice.

That doesn't sound like the right approach to me -- Remarks are usually for 
reporting backend decision-making to the frontend for things like optimization 
passes. In this case, it may make more sense to make it a `Warning<>` but make 
it `DefaultIgnore` so that you have to enable it explicitly.

Btw, it looks like when you generated this patch, it was generated against the 
previous diff and not trunk, so the diff view is misleading. When you update 
the patch, can you be sure to diff against the trunk?


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

https://reviews.llvm.org/D87962

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

[PATCH] D87962: [clang] Change the multi-character character constants from extension to implementation-defined.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> Or should we change the four character case to "Remark" so it wouldn't be 
> warned even with the "-pandemic" option? There seems no other choice.

There's a DefaultIgnore modifier for warnings that turns then off by default.  
We use this sparingly, since it's hard to discover off-by-default warnings, but 
seems appropriate here.


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

https://reviews.llvm.org/D87962

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

[PATCH] D54222: [clang-tidy] Add a check to detect returning static locals in public headers

[Joe Ranieri via Phabricator via cfe-commits]

jranieri-grammatech updated this revision to Diff 295563.
jranieri-grammatech added a comment.
Herald added a project: clang.

In D54222#2295374 , @JonasToth wrote:

> Do you have data for other projects? As this is not a very common thing and 
> probably different for code-bases with plugins and so on, the "chattiness" of 
> the check would be interesting to know.

I ran it on our large internal codebase. It generated one unique warning (2 
total) with the filter on `/include/`. It was a false positive but at least it 
was actually complaining about something plugin-related. When loosening it up 
to any included file, that grows to five unique warnings (231 total) and all of 
the additional warnings were false positives.

> If the check is usually quiet, then i would think its ok to have it as a 
> general check together with proper documentation explaining why these statics 
> can be a problem.

I think the fact that it's triggered in header files makes it inherently 
noisier than other checkers. The warnings in LLVM I pasted above were only the 
unique instances -- there were a total of 323 warnings issued for the first set 
and a total of 353 warnings issued when relaxing the restrictions.

> I would still like to have it in `bugprone-`, because this is a real problem 
> that can arise and the check is more likely to be ignored if considered to be 
> "just for llvm".

I've updated the patch and moved it into `bugprone-`. I think it probably needs 
a better name than `bugprone-problematic-statics`, now that it's expected to be 
used outside of the LLVM context.

> The decision for true vs false positive is not possible to decide within 
> clang-tidy, is it? I think this should then be left to the developer (as it 
> is probably not that much?).

It can't be decided completely. I think that this checker would be more useful 
with a configuration option to specify a regex for "plugin API headers", but I 
don't have the time to implement it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54222

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/ProblematicStaticsCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/ProblematicStaticsCheck.h
  clang-tools-extra/docs/clang-tidy/checks/bugprone-problematic-statics.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-problematic-statics/bugprone-problematic-statics.h
  clang-tools-extra/test/clang-tidy/checkers/bugprone-problematic-statics.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-problematic-statics.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-problematic-statics.cpp
@@ -0,0 +1,11 @@
+// RUN: %check_clang_tidy %s bugprone-problematic-statics %t -- -header-filter='.*' -- -I %S/Inputs/bugprone-problematic-statics
+
+#include "bugprone-problematic-statics.h"
+// CHECK-MESSAGES: bugprone-problematic-statics.h:7:5: warning: address of static local variable 'index' may not be identical across library boundaries [bugprone-problematic-statics]
+
+struct ProgramStateTraitInMain {
+  static void *GDMIndex() {
+static int index = 0;
+return 
+  }
+};
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-problematic-statics/bugprone-problematic-statics.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-problematic-statics/bugprone-problematic-statics.h
@@ -0,0 +1,11 @@
+#ifndef PROBLEMATIC_STATICS_H
+#define PROBLEMATIC_STATICS_H
+
+struct ProgramStateTrait {
+  static void *GDMIndex() {
+static int index = 0;
+return 
+  }
+};
+
+#endif
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -75,6 +75,7 @@
`bugprone-not-null-terminated-result `_, "Yes"
`bugprone-parent-virtual-call `_, "Yes"
`bugprone-posix-return `_, "Yes"
+   `bugprone-problematic-statics `_,
`bugprone-redundant-branch-condition `_, "Yes"
`bugprone-reserved-identifier `_, "Yes"
`bugprone-signed-char-misuse `_,
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-problematic-statics.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-problematic-statics.rst
@@ -0,0 +1,17 @@
+.. title:: clang-tidy - bugprone-problematic-statics
+
+bugprone-problematic-statics
+
+
+Detects functions defined in headers that return the address of a static
+local 

[clang-tools-extra] f6b1323 - Reland [clangd] clangd --check: standalone diagnosis of common problems

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-10-01T16:18:18+02:00
New Revision: f6b1323bc680812e04904293854c356530985bcd

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

LOG: Reland [clangd] clangd --check: standalone diagnosis of common problems

This reverts commit 30d07b14a274f075a01d201ad59723ca1a4a9b57.

Test failures have (hopefully) been fixed.

Added: 
clang-tools-extra/clangd/test/check-fail.test
clang-tools-extra/clangd/test/check.test
clang-tools-extra/clangd/tool/Check.cpp

Modified: 
clang-tools-extra/clangd/tool/CMakeLists.txt
clang-tools-extra/clangd/tool/ClangdMain.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/test/check-fail.test 
b/clang-tools-extra/clangd/test/check-fail.test
new file mode 100644
index ..0ee777f02cc5
--- /dev/null
+++ b/clang-tools-extra/clangd/test/check-fail.test
@@ -0,0 +1,14 @@
+// RUN: cp %s %t.cpp
+// RUN: not clangd -check=%t.cpp 2>&1 | FileCheck -strict-whitespace %s
+
+// CHECK: Testing on source file {{.*}}check-fail.test
+// CHECK: internal (cc1) args are: -cc1
+// CHECK: Building preamble...
+// CHECK: [pp_file_not_found] Line {{.*}}: 'missing.h' file not found
+// CHECK: Building AST...
+// CHECK: Testing features at each token
+// CHECK: tweak: ExpandAutoType ==> FAIL
+// CHECK: All checks completed, 2 errors
+
+#include "missing.h"
+auto x = []{};

diff  --git a/clang-tools-extra/clangd/test/check.test 
b/clang-tools-extra/clangd/test/check.test
new file mode 100644
index ..832629ce29ef
--- /dev/null
+++ b/clang-tools-extra/clangd/test/check.test
@@ -0,0 +1,13 @@
+# RUN: clangd -log=verbose -check 2>&1 | FileCheck -strict-whitespace %s
+
+CHECK: Testing on source file {{.*}}test.cc
+CHECK: internal (cc1) args are: -cc1
+CHECK: Building preamble...
+CHECK: Built preamble
+CHECK: Building AST...
+CHECK: Testing features at each token
+CHECK-DAG: hover: false
+CHECK-DAG: hover: true
+CHECK-DAG: tweak: AddUsing
+CHECK: All checks completed, 0 errors
+

diff  --git a/clang-tools-extra/clangd/tool/CMakeLists.txt 
b/clang-tools-extra/clangd/tool/CMakeLists.txt
index 670e5a17013a..65e0aa35f265 100644
--- a/clang-tools-extra/clangd/tool/CMakeLists.txt
+++ b/clang-tools-extra/clangd/tool/CMakeLists.txt
@@ -3,6 +3,7 @@ include_directories(${CMAKE_CURRENT_BINARY_DIR}/..)
 
 add_clang_tool(clangd
   ClangdMain.cpp
+  Check.cpp
   $
   )
 

diff  --git a/clang-tools-extra/clangd/tool/Check.cpp 
b/clang-tools-extra/clangd/tool/Check.cpp
new file mode 100644
index ..14ee0fdec9c9
--- /dev/null
+++ b/clang-tools-extra/clangd/tool/Check.cpp
@@ -0,0 +1,258 @@
+//===--- Check.cpp - clangd self-diagnostics 
--===//
+//
+// 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
+//
+//===--===//
+//
+// Many basic problems can occur processing a file in clangd, e.g.:
+//  - system includes are not found
+//  - crash when indexing its AST
+// clangd --check provides a simplified, isolated way to reproduce these,
+// with no editor, LSP, threads, background indexing etc to contend with.
+//
+// One important use case is gathering information for bug reports.
+// Another is reproducing crashes, and checking which setting prevent them.
+//
+// It simulates opening a file (determining compile command, parsing, indexing)
+// and then running features at many locations.
+//
+// Currently it adds some basic logging of progress and results.
+// We should consider extending it to also recognize common symptoms and
+// recommend solutions (e.g. standard library installation issues).
+//
+//===--===//
+
+#include "ClangdLSPServer.h"
+#include "CodeComplete.h"
+#include "GlobalCompilationDatabase.h"
+#include "Hover.h"
+#include "ParsedAST.h"
+#include "Preamble.h"
+#include "SourceCode.h"
+#include "XRefs.h"
+#include "index/CanonicalIncludes.h"
+#include "index/FileIndex.h"
+#include "refactor/Tweak.h"
+#include "support/ThreadsafeFS.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Format/Format.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Path.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+// Print (and count) the error-level diagnostics (warnings are ignored).
+unsigned showErrors(llvm::ArrayRef Diags) {
+  unsigned ErrCount = 0;
+  for (const auto  : Diags) {
+if 

[PATCH] D88338: [clangd] clangd --check: standalone diagnosis of common problems

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks, I think this fails whenever `compile_commands.json` doesn't exist in 
the tree or under `build`.
(Which of course it does in my local checkout...). Reverted, probably need to 
rename/copy the test file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88338

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

[PATCH] D88275: [ASTMatchers] Add matcher `hasParentIgnoringImplicit`.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D88275#2303283 , @ymandel wrote:

>> I'm not concerned about the basic idea behind the proposed matcher, I'm only 
>> worried we're making AST matching more confusing by having two different 
>> ways of inconsistently accomplishing the same-ish thing.
>
> Aaron, I appreciate this concern, but I would argue that this matcher isn't 
> making things any worse. We already have the various `ignoringImplicit` 
> matchers, and this new one simply parallels those, but for parents. So, it is 
> in some sense "completing" an existing API, which together is an alternative 
> to  `traverse`.

I'm not certain I agree with that reasoning because you can extend it to 
literally any match that may interact with implicit nodes, which is the whole 
point to the spelled in source traversal mode. I'm not certain it's a good 
design for us to continue to add one-off ways to ignore implicit nodes.

> We should decide our general policy on these implict matchers vs the traverse 
> matchers.

I agree.

> Personally, I view `traverse` as an experimental feature that we're still 
> figuring out and so would prefer that it not block progress on new matchers. 
> But, I'm open to discussion. I can implement this one in my own codebase in 
> the meantime if this requires longer discussion (that is, it's not blocking 
> me, fwiw).

FWIW, I think of `traverse` as experimental as well. I can see the argument for 
not letting an experimental feature block progress on new matchers, too. I'm 
mostly worried about the case where we add the new matches and keep the 
`traverse` API, but they have subtly different behaviors and no clear policy on 
when to use which form.

> Also, I don't believe that traverse work in this case. When I change the test 
> to use `traverse`, it fails:
>
>   TEST(HasParentIgnoringImplicit, TraverseMatchesExplicitParents) {
> std::string Input = R"cc(
>   float f() {
>   int x = 3;
>   int y = 3.0;
>   return y;
>   }
> )cc";
>  // ---> Passes because there are no implicit parents.
> EXPECT_TRUE(matches(
> Input, integerLiteral(traverse(TK_IgnoreImplicitCastsAndParentheses,
>hasParent(varDecl());
> // Fails
> EXPECT_TRUE(
> matches(Input, 
> declRefExpr(traverse(TK_IgnoreImplicitCastsAndParentheses,
> hasParent(returnStmt());
> // Fails
> EXPECT_TRUE(
> matches(Input, 
> floatLiteral(traverse(TK_IgnoreImplicitCastsAndParentheses,
>  hasParent(varDecl());
>   }

Interesting catch and not the behavior I would expect from `traverse`. 
@steveire, would you consider this to be a bug in the traversal behavior?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88275

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

[clang-tools-extra] 30d07b1 - Revert "[clangd] clangd --check: standalone diagnosis of common problems"

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-10-01T16:10:03+02:00
New Revision: 30d07b14a274f075a01d201ad59723ca1a4a9b57

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

LOG: Revert "[clangd] clangd --check: standalone diagnosis of common problems"

This reverts commit 79fbcbff41734e3d07e6200d33c3e40732dfae6a.

The fallback command fails to parse for the test files if there's no
compile_commands.json in the tree.

Added: 


Modified: 
clang-tools-extra/clangd/tool/CMakeLists.txt
clang-tools-extra/clangd/tool/ClangdMain.cpp

Removed: 
clang-tools-extra/clangd/test/check-fail.test
clang-tools-extra/clangd/test/check.test
clang-tools-extra/clangd/tool/Check.cpp



diff  --git a/clang-tools-extra/clangd/test/check-fail.test 
b/clang-tools-extra/clangd/test/check-fail.test
deleted file mode 100644
index 7462ce5ecf5f..
--- a/clang-tools-extra/clangd/test/check-fail.test
+++ /dev/null
@@ -1,13 +0,0 @@
-// RUN: not clangd -check=%s 2>&1 | FileCheck -strict-whitespace %s
-
-// CHECK: Testing on source file {{.*}}check-fail.test
-// CHECK: internal (cc1) args are: -cc1
-// CHECK: Building preamble...
-// CHECK: [pp_file_not_found] Line {{.*}}: 'missing.h' file not found
-// CHECK: Building AST...
-// CHECK: Testing features at each token
-// CHECK: tweak: ExpandAutoType ==> FAIL
-// CHECK: All checks completed, 2 errors
-
-#include "missing.h"
-auto x = []{};

diff  --git a/clang-tools-extra/clangd/test/check.test 
b/clang-tools-extra/clangd/test/check.test
deleted file mode 100644
index 832629ce29ef..
--- a/clang-tools-extra/clangd/test/check.test
+++ /dev/null
@@ -1,13 +0,0 @@
-# RUN: clangd -log=verbose -check 2>&1 | FileCheck -strict-whitespace %s
-
-CHECK: Testing on source file {{.*}}test.cc
-CHECK: internal (cc1) args are: -cc1
-CHECK: Building preamble...
-CHECK: Built preamble
-CHECK: Building AST...
-CHECK: Testing features at each token
-CHECK-DAG: hover: false
-CHECK-DAG: hover: true
-CHECK-DAG: tweak: AddUsing
-CHECK: All checks completed, 0 errors
-

diff  --git a/clang-tools-extra/clangd/tool/CMakeLists.txt 
b/clang-tools-extra/clangd/tool/CMakeLists.txt
index 65e0aa35f265..670e5a17013a 100644
--- a/clang-tools-extra/clangd/tool/CMakeLists.txt
+++ b/clang-tools-extra/clangd/tool/CMakeLists.txt
@@ -3,7 +3,6 @@ include_directories(${CMAKE_CURRENT_BINARY_DIR}/..)
 
 add_clang_tool(clangd
   ClangdMain.cpp
-  Check.cpp
   $
   )
 

diff  --git a/clang-tools-extra/clangd/tool/Check.cpp 
b/clang-tools-extra/clangd/tool/Check.cpp
deleted file mode 100644
index 14ee0fdec9c9..
--- a/clang-tools-extra/clangd/tool/Check.cpp
+++ /dev/null
@@ -1,258 +0,0 @@
-//===--- Check.cpp - clangd self-diagnostics 
--===//
-//
-// 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
-//
-//===--===//
-//
-// Many basic problems can occur processing a file in clangd, e.g.:
-//  - system includes are not found
-//  - crash when indexing its AST
-// clangd --check provides a simplified, isolated way to reproduce these,
-// with no editor, LSP, threads, background indexing etc to contend with.
-//
-// One important use case is gathering information for bug reports.
-// Another is reproducing crashes, and checking which setting prevent them.
-//
-// It simulates opening a file (determining compile command, parsing, indexing)
-// and then running features at many locations.
-//
-// Currently it adds some basic logging of progress and results.
-// We should consider extending it to also recognize common symptoms and
-// recommend solutions (e.g. standard library installation issues).
-//
-//===--===//
-
-#include "ClangdLSPServer.h"
-#include "CodeComplete.h"
-#include "GlobalCompilationDatabase.h"
-#include "Hover.h"
-#include "ParsedAST.h"
-#include "Preamble.h"
-#include "SourceCode.h"
-#include "XRefs.h"
-#include "index/CanonicalIncludes.h"
-#include "index/FileIndex.h"
-#include "refactor/Tweak.h"
-#include "support/ThreadsafeFS.h"
-#include "clang/AST/ASTContext.h"
-#include "clang/Basic/DiagnosticIDs.h"
-#include "clang/Format/Format.h"
-#include "clang/Frontend/CompilerInvocation.h"
-#include "clang/Tooling/CompilationDatabase.h"
-#include "llvm/ADT/ArrayRef.h"
-#include "llvm/ADT/Optional.h"
-#include "llvm/ADT/StringExtras.h"
-#include "llvm/Support/Path.h"
-
-namespace clang {
-namespace clangd {
-namespace {
-
-// Print (and count) the error-level diagnostics (warnings are ignored).
-unsigned showErrors(llvm::ArrayRef Diags) {
-  unsigned ErrCount = 

[PATCH] D88566: be more specific when testing for no fuse-ld warnings

[Dave Green via Phabricator via cfe-commits]

dmgreen added a comment.

(Not that I'm against this, either way sounds fine to me)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88566

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

[PATCH] D88338: [clangd] clangd --check: standalone diagnosis of common problems

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Might break tests: http://45.33.8.238/linux/29238/step_9.txt


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88338

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

[PATCH] D88403: Migrate Declarators to use the List API

[Eduardo Caldas via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5011d43108d1: Migrate Declarators to use the List API 
(authored by eduucaldas).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88403

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/lib/Tooling/Syntax/Synthesis.cpp
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
  clang/unittests/Tooling/Syntax/SynthesisTest.cpp

Index: clang/unittests/Tooling/Syntax/SynthesisTest.cpp
===
--- clang/unittests/Tooling/Syntax/SynthesisTest.cpp
+++ clang/unittests/Tooling/Syntax/SynthesisTest.cpp
@@ -188,8 +188,9 @@
 TranslationUnit Detached synthesized
 `-SimpleDeclaration synthesized
   |-'int' synthesized
-  |-SimpleDeclarator Declarator synthesized
-  | `-'a' synthesized
+  |-DeclaratorList Declarators synthesized
+  | `-SimpleDeclarator ListElement synthesized
+  |   `-'a' synthesized
   `-';' synthesized
   )txt"));
 }
@@ -201,8 +202,9 @@
   EXPECT_TRUE(treeDumpEqual(Copy, R"txt(
 SimpleDeclaration Detached synthesized
 |-'int' synthesized
-|-SimpleDeclarator Declarator synthesized
-| `-'a' synthesized
+|-DeclaratorList Declarators synthesized
+| `-SimpleDeclarator ListElement synthesized
+|   `-'a' synthesized
 `-';' synthesized
   )txt"));
 }
@@ -225,11 +227,12 @@
 TranslationUnit Detached synthesized
 `-SimpleDeclaration synthesized
   |-'void' synthesized
-  |-SimpleDeclarator Declarator synthesized
-  | |-'test' synthesized
-  | `-ParametersAndQualifiers synthesized
-  |   |-'(' OpenParen synthesized
-  |   `-')' CloseParen synthesized
+  |-DeclaratorList Declarators synthesized
+  | `-SimpleDeclarator ListElement synthesized
+  |   |-'test' synthesized
+  |   `-ParametersAndQualifiers synthesized
+  | |-'(' OpenParen synthesized
+  | `-')' CloseParen synthesized
   `-CompoundStatement synthesized
 |-'{' OpenParen synthesized
 |-IfStatement Statement synthesized
Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -92,21 +92,23 @@
 TranslationUnit Detached
 |-SimpleDeclaration
 | |-'int'
-| |-SimpleDeclarator Declarator
-| | |-'main'
-| | `-ParametersAndQualifiers
-| |   |-'(' OpenParen
-| |   `-')' CloseParen
+| |-DeclaratorList Declarators
+| | `-SimpleDeclarator ListElement
+| |   |-'main'
+| |   `-ParametersAndQualifiers
+| | |-'(' OpenParen
+| | `-')' CloseParen
 | `-CompoundStatement
 |   |-'{' OpenParen
 |   `-'}' CloseParen
 `-SimpleDeclaration
   |-'void'
-  |-SimpleDeclarator Declarator
-  | |-'foo'
-  | `-ParametersAndQualifiers
-  |   |-'(' OpenParen
-  |   `-')' CloseParen
+  |-DeclaratorList Declarators
+  | `-SimpleDeclarator ListElement
+  |   |-'foo'
+  |   `-ParametersAndQualifiers
+  | |-'(' OpenParen
+  | `-')' CloseParen
   `-CompoundStatement
 |-'{' OpenParen
 `-'}' CloseParen
@@ -123,16 +125,18 @@
 TranslationUnit Detached
 |-SimpleDeclaration
 | |-'int'
-| |-SimpleDeclarator Declarator
-| | `-'a'
+| |-DeclaratorList Declarators
+| | `-SimpleDeclarator ListElement
+| |   `-'a'
 | `-';'
 `-SimpleDeclaration
   |-'int'
-  |-SimpleDeclarator Declarator
-  | |-'b'
-  | |-'='
-  | `-IntegerLiteralExpression
-  |   `-'42' LiteralToken
+  |-DeclaratorList Declarators
+  | `-SimpleDeclarator ListElement
+  |   |-'b'
+  |   |-'='
+  |   `-IntegerLiteralExpression
+  | `-'42' LiteralToken
   `-';'
 )txt"));
 }
@@ -146,21 +150,24 @@
 TranslationUnit Detached
 `-SimpleDeclaration
   |-'void'
-  |-SimpleDeclarator Declarator
-  | |-'foo'
-  | `-ParametersAndQualifiers
-  |   |-'(' OpenParen
-  |   |-ParameterDeclarationList Parameters
-  |   | |-SimpleDeclaration ListElement
-  |   | | |-'int'
-  |   | | `-SimpleDeclarator Declarator
-  |   | |   `-'a'
-  |   | |-',' ListDelimiter
-  |   | `-SimpleDeclaration ListElement
-  |   |   |-'int'
-  |   |   `-SimpleDeclarator Declarator
-  |   | `-'b'
-  |   `-')' CloseParen
+  |-DeclaratorList Declarators
+  | `-SimpleDeclarator ListElement
+  |   |-'foo'
+  |   `-ParametersAndQualifiers
+  | |-'(' OpenParen
+  | |-ParameterDeclarationList Parameters
+  | | |-SimpleDeclaration ListElement
+  | | | |-'int'
+  | | | `-DeclaratorList Declarators
+  | | |   `-SimpleDeclarator ListElement
+  | | | `-'a'
+  | | |-',' ListDelimiter
+  | | `-SimpleDeclaration ListElement
+  | |   |-'int'
+  | |   `-DeclaratorList Declarators
+  | | `-SimpleDeclarator ListElement
+  | |   `-'b'
+  | `-')' CloseParen
   `-CompoundStatement
 |-'{' OpenParen
 `-'}' CloseParen
@@ -178,8 +185,9 @@
 `-SimpleDeclaration
   |-'in\
 t'
-  |-SimpleDeclarator Declarator

[PATCH] D88566: be more specific when testing for no fuse-ld warnings

[Dave Green via Phabricator via cfe-commits]

dmgreen added a comment.

In the past we have usually disabled the downstream warning for similar 
catch-all warning lines.




Comment at: clang/test/Driver/fuse-ld.c:5
 // RUN:   FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
 // CHECK-ABSOLUTE-LD: warning: '-fuse-ld=' taking a path is deprecated. Use 
'--ld-path=' instead
 // CHECK-ABSOLUTE-LD: /usr/local/bin/or1k-linux-ld

This line has a = on the end. Would the other warning below also have that? 
(Negative tests are often tricky).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88566

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

[clang] 5011d43 - Migrate Declarators to use the List API

[Eduardo Caldas via cfe-commits]

Author: Eduardo Caldas
Date: 2020-10-01T13:56:31Z
New Revision: 5011d43108d1de30a056d66e73fa19062e0e84b7

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

LOG: Migrate Declarators to use the List API

After this change all nodes that have a delimited-list are using the
`List` API.

Implementation details:
Let's look at a declaration with multiple declarators:
`int a, b;`
To generate a declarator list node we need to have the range of
declarators: `a, b`:
However, the `ClangAST` actually stores them as separate declarations:
`int a   ;`
`intb;`
We solve that by appropriately marking the declarators on each separate
declaration in the `ClangAST` and then for the final declarator `int
b`, shrinking its range to fit to the already marked declarators.

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

Added: 


Modified: 
clang/include/clang/Tooling/Syntax/Nodes.h
clang/lib/Tooling/Syntax/BuildTree.cpp
clang/lib/Tooling/Syntax/Nodes.cpp
clang/lib/Tooling/Syntax/Synthesis.cpp
clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
clang/unittests/Tooling/Syntax/SynthesisTest.cpp

Removed: 




diff  --git a/clang/include/clang/Tooling/Syntax/Nodes.h 
b/clang/include/clang/Tooling/Syntax/Nodes.h
index 8b393c5423b4..ed4449adb0f0 100644
--- a/clang/include/clang/Tooling/Syntax/Nodes.h
+++ b/clang/include/clang/Tooling/Syntax/Nodes.h
@@ -99,10 +99,14 @@ enum class NodeKind : uint16_t {
   ParametersAndQualifiers,
   MemberPointer,
   UnqualifiedId,
+
+  // Lists
+  DeclaratorList,
   ParameterDeclarationList,
   CallArguments,
-  // Nested Name Specifiers.
   NestedNameSpecifier,
+
+  // Name Specifiers.
   GlobalNameSpecifier,
   DecltypeNameSpecifier,
   IdentifierNameSpecifier,
@@ -179,6 +183,7 @@ enum class NodeRole : uint8_t {
   Member,
   Callee,
   Arguments,
+  Declarators
 };
 /// For debugging purposes.
 raw_ostream <<(raw_ostream , NodeRole R);
@@ -823,6 +828,17 @@ class LinkageSpecificationDeclaration final : public 
Declaration {
   }
 };
 
+class DeclaratorList final : public List {
+public:
+  DeclaratorList() : List(NodeKind::DeclaratorList) {}
+  static bool classof(const Node *N) {
+return N->getKind() == NodeKind::DeclaratorList;
+  }
+  std::vector getDeclarators();
+  std::vector>
+  getDeclaratorsAndCommas();
+};
+
 /// Groups multiple declarators (e.g. variables, typedefs, etc.) together. All
 /// grouped declarators share the same declaration specifiers (e.g. 'int' or
 /// 'typedef').

diff  --git a/clang/lib/Tooling/Syntax/BuildTree.cpp 
b/clang/lib/Tooling/Syntax/BuildTree.cpp
index 4d365090abf1..e1ed55f2e4eb 100644
--- a/clang/lib/Tooling/Syntax/BuildTree.cpp
+++ b/clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -397,6 +397,17 @@ class syntax::TreeBuilder {
   Mapping.add(From, New);
   }
 
+  /// Populate children for \p New list, assuming it covers tokens from a
+  /// subrange of \p SuperRange.
+  void foldList(ArrayRef SuperRange, syntax::List *New,
+ASTPtr From) {
+assert(New);
+auto ListRange = Pending.shrinkToFitList(SuperRange);
+Pending.foldChildren(Arena, ListRange, New);
+if (From)
+  Mapping.add(From, New);
+  }
+
   /// Notifies that we should not consume trailing semicolon when computing
   /// token range of \p D.
   void noticeDeclWithoutSemicolon(Decl *D);
@@ -579,6 +590,35 @@ class syntax::TreeBuilder {
   It->second->setRole(Role);
 }
 
+/// Shrink \p Range to a subrange that only contains tokens of a list.
+/// List elements and delimiters should already have correct roles.
+ArrayRef shrinkToFitList(ArrayRef Range) {
+  auto BeginChildren = Trees.lower_bound(Range.begin());
+  assert((BeginChildren == Trees.end() ||
+  BeginChildren->first == Range.begin()) &&
+ "Range crosses boundaries of existing subtrees");
+
+  auto EndChildren = Trees.lower_bound(Range.end());
+  assert(
+  (EndChildren == Trees.end() || EndChildren->first == Range.end()) &&
+  "Range crosses boundaries of existing subtrees");
+
+  auto BelongsToList = [](decltype(Trees)::value_type KV) {
+auto Role = KV.second->getRole();
+return Role == syntax::NodeRole::ListElement ||
+   Role == syntax::NodeRole::ListDelimiter;
+  };
+
+  auto BeginListChildren =
+  std::find_if(BeginChildren, EndChildren, BelongsToList);
+
+  auto EndListChildren =
+  std::find_if_not(BeginListChildren, EndChildren, BelongsToList);
+
+  return ArrayRef(BeginListChildren->first,
+ EndListChildren->first);
+}
+
 /// Add \p Node to the forest and attach child nodes based on \p Tokens.
 void foldChildren(const syntax::Arena , ArrayRef Tokens,
 

[PATCH] D87962: [clang] Change the multi-character character constants from extension to implementation-defined.

[Chuyang Chen via Phabricator via cfe-commits]

nomanous updated this revision to Diff 295559.

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

https://reviews.llvm.org/D87962

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/test/CXX/lex/lex.literal/lex.ccon/p1.cpp
  clang/test/FixIt/format.m
  clang/test/Lexer/char-literal.cpp
  clang/test/Lexer/constants.c
  clang/test/Lexer/multi-character-character-constant.c
  clang/test/Preprocessor/expr_multichar.c

Index: clang/test/Preprocessor/expr_multichar.c
===
--- clang/test/Preprocessor/expr_multichar.c
+++ clang/test/Preprocessor/expr_multichar.c
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 < %s -E -verify -triple i686-pc-linux-gnu
-// Expect the warning of the four-character character constant after changing it from extension to implementation-defined
+// expected-no-diagnostics
 
-#if (('1234' >> 24) != '1') // expected-warning {{multi-character character constant}}
+#if (('1234' >> 24) != '1')
 #error Bad multichar constant calculation!
 #endif
Index: clang/test/Lexer/multi-character-character-constant.c
===
--- clang/test/Lexer/multi-character-character-constant.c
+++ clang/test/Lexer/multi-character-character-constant.c
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -pedantic-errors %s
 
 int x = 'ab'; // expected-warning {{multi-character character constant}}
-int y = 'abcd'; // expected-warning {{multi-character character constant}}
+int y = 'abcd'; // no warning.
 
 int main() {
return 0;
Index: clang/test/Lexer/constants.c
===
--- clang/test/Lexer/constants.c
+++ clang/test/Lexer/constants.c
@@ -26,7 +26,7 @@
   '\\
 t',
   '??!',  // expected-warning {{trigraph converted to '|' character}}
-  'abcd'  // expected-warning {{multi-character character constant}}
+  'abcd'
 };
 
 //  PR4499
@@ -36,15 +36,16 @@
 int m3 = '\\\
 ';
 
+// The four-character character constants wouldn't cause warning anymore after we change it to "Remark"
 
 #pragma clang diagnostic ignored "-Wmultichar"
 
 int d = 'df'; // no warning.
-int e = 'abcd';  // still warn: expected-warning {{multi-character character constant}}
+int e = 'abcd';
 
-#pragma clang diagnostic ignored "-Wfour-char-constants"
+// #pragma clang diagnostic ignored "-Wfour-char-constants"
 
-int f = 'abcd';  // ignored.
+int f = 'abcd';
 
 // rdar://problem/6974641
 float t0[] = {
Index: clang/test/Lexer/char-literal.cpp
===
--- clang/test/Lexer/char-literal.cpp
+++ clang/test/Lexer/char-literal.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -Wfour-char-constants -fsyntax-only -verify %s
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c11 -x c -Wfour-char-constants -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c11 -x c -fsyntax-only -verify %s
 
 #ifndef __cplusplus
 typedef __WCHAR_TYPE__ wchar_t;
@@ -9,7 +9,7 @@
 
 int a = 'ab'; // expected-warning {{multi-character character constant}}
 int b = '\xFF\xFF'; // expected-warning {{multi-character character constant}}
-int c = 'APPS'; // expected-warning {{multi-character character constant}}
+int c = 'APPS';
 
 char d = '鈱?; // expected-error {{character too large for enclosing character literal type}}
 char e = '\u2318'; // expected-error {{character too large for enclosing character literal type}}
Index: clang/test/FixIt/format.m
===
--- clang/test/FixIt/format.m
+++ clang/test/FixIt/format.m
@@ -162,16 +162,13 @@
 
 
   NSLog(@"%s", 'abcd'); // expected-warning{{format specifies type 'char *' but the argument has type 'int'}} \
-  // expected-warning {{multi-character character constant}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:11-[[@LINE-2]]:13}:"%d"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%d"
 
   NSLog(@"%lf", 'abcd'); // expected-warning{{format specifies type 'double' but the argument has type 'int'}} \
-  // expected-warning {{multi-character character constant}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:11-[[@LINE-2]]:14}:"%d"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:14}:"%d"
 
   NSLog(@"%@", 'abcd'); // expected-warning{{format specifies type 'id' but the argument has type 'int'}} \
-  // expected-warning {{multi-character character constant}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:11-[[@LINE-2]]:13}:"%d"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%d"
 }
 
 void multichar_constants_false_negative() {
@@ -181,8 +178,7 @@
   // many C library functions like fgetc() actually return an int (using -1
   // as a sentinel).
   NSLog(@"%c", 'abcd'); // missing-warning{{format specifies type 'char' but the argument has type 

[PATCH] D88338: [clangd] clangd --check: standalone diagnosis of common problems

[Sam McCall via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rG79fbcbff4173: [clangd] clangd --check: standalone diagnosis 
of common problems (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D88338?vs=295290=295558#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88338

Files:
  clang-tools-extra/clangd/test/check-fail.test
  clang-tools-extra/clangd/test/check.test
  clang-tools-extra/clangd/tool/CMakeLists.txt
  clang-tools-extra/clangd/tool/Check.cpp
  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
@@ -47,6 +47,11 @@
 
 namespace clang {
 namespace clangd {
+
+// Implemented in Check.cpp.
+bool check(const llvm::StringRef File, const ThreadsafeFS ,
+   const ClangdLSPServer::Options );
+
 namespace {
 
 using llvm::cl::cat;
@@ -57,6 +62,7 @@
 using llvm::cl::list;
 using llvm::cl::opt;
 using llvm::cl::OptionCategory;
+using llvm::cl::ValueOptional;
 using llvm::cl::values;
 
 // All flags must be placed in a category, or they will be shown neither in
@@ -354,6 +360,16 @@
 Hidden,
 };
 
+opt CheckFile{
+"check",
+cat(Misc),
+desc("Parse one file in isolation instead of acting as a language server. "
+ "Useful to investigate/reproduce crashes or configuration problems. "
+ "With --check=, attempts to parse a particular file."),
+init(""),
+ValueOptional,
+};
+
 enum PCHStorageFlag { Disk, Memory };
 opt PCHStorage{
 "pch-storage",
@@ -541,7 +557,8 @@
 
 enum class ErrorResultCode : int {
   NoShutdownRequest = 1,
-  CantRunAsXPCService = 2
+  CantRunAsXPCService = 2,
+  CheckFailed = 3
 };
 
 int main(int argc, char *argv[]) {
@@ -646,7 +663,8 @@
   // If a user ran `clangd` in a terminal without redirecting anything,
   // it's somewhat likely they're confused about how to use clangd.
   // Show them the help overview, which explains.
-  if (llvm::outs().is_displayed() && llvm::errs().is_displayed())
+  if (llvm::outs().is_displayed() && llvm::errs().is_displayed() &&
+  !CheckFile.getNumOccurrences())
 llvm::errs() << Overview << "\n";
   // Use buffered stream to stderr (we still flush each log message). Unbuffered
   // stream can cause significant (non-deterministic) latency for the logger.
@@ -825,6 +843,15 @@
   // Shall we allow to customize the file limit?
   Opts.Rename.AllowCrossFile = CrossFileRename;
 
+  if (CheckFile.getNumOccurrences()) {
+llvm::SmallString<256> Path;
+llvm::sys::fs::real_path(CheckFile, Path, /*expand_tilde=*/true);
+log("Entering check mode (no LSP server)");
+return check(Path, TFS, Opts)
+   ? 0
+   : static_cast(ErrorResultCode::CheckFailed);
+  }
+
   // Initialize and run ClangdLSPServer.
   // Change stdin to binary to not lose \r\n on windows.
   llvm::sys::ChangeStdinToBinary();
@@ -835,7 +862,7 @@
 TransportLayer = newXPCTransport();
 #else
 llvm::errs() << "This clangd binary wasn't built with XPC support.\n";
-return (int)ErrorResultCode::CantRunAsXPCService;
+return static_cast(ErrorResultCode::CantRunAsXPCService);
 #endif
   } else {
 log("Starting LSP over stdin/stdout");
Index: clang-tools-extra/clangd/tool/Check.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -0,0 +1,258 @@
+//===--- Check.cpp - clangd self-diagnostics --===//
+//
+// 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
+//
+//===--===//
+//
+// Many basic problems can occur processing a file in clangd, e.g.:
+//  - system includes are not found
+//  - crash when indexing its AST
+// clangd --check provides a simplified, isolated way to reproduce these,
+// with no editor, LSP, threads, background indexing etc to contend with.
+//
+// One important use case is gathering information for bug reports.
+// Another is reproducing crashes, and checking which setting prevent them.
+//
+// It simulates opening a file (determining compile command, parsing, indexing)
+// and then running features at many locations.
+//
+// Currently it adds some basic logging of progress and results.
+// We should consider extending it to also recognize common symptoms and
+// recommend solutions (e.g. standard library installation issues).
+//

[clang-tools-extra] 79fbcbf - [clangd] clangd --check: standalone diagnosis of common problems

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-10-01T15:47:47+02:00
New Revision: 79fbcbff41734e3d07e6200d33c3e40732dfae6a

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

LOG: [clangd] clangd --check: standalone diagnosis of common problems

This is a tool to simply parse a file as clangd would, and run some
common features (code actions, go-to-definition, hover) in an attempt to
trigger or reproduce crashes, error diagnostics, etc.

This is easier and more predictable than loading the file in clangd, because:
 - there's no editor/plugin variation to worry about
 - there's no accidental variation of user behavior or other extraneous requests
 - we trigger features at every token, rather than guessing
 - everything is synchronoous, logs are easier to reason about
 - it's easier to (get users to) capture logs when running on the command-line

This is a fairly lightweight variant of this idea.
We could do a lot more with it, and maybe we should.
But I can't in the near future, and experience will tell us if we made
the right tradeoffs and if it's worth investing further.

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

Added: 
clang-tools-extra/clangd/test/check-fail.test
clang-tools-extra/clangd/test/check.test
clang-tools-extra/clangd/tool/Check.cpp

Modified: 
clang-tools-extra/clangd/tool/CMakeLists.txt
clang-tools-extra/clangd/tool/ClangdMain.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/test/check-fail.test 
b/clang-tools-extra/clangd/test/check-fail.test
new file mode 100644
index ..7462ce5ecf5f
--- /dev/null
+++ b/clang-tools-extra/clangd/test/check-fail.test
@@ -0,0 +1,13 @@
+// RUN: not clangd -check=%s 2>&1 | FileCheck -strict-whitespace %s
+
+// CHECK: Testing on source file {{.*}}check-fail.test
+// CHECK: internal (cc1) args are: -cc1
+// CHECK: Building preamble...
+// CHECK: [pp_file_not_found] Line {{.*}}: 'missing.h' file not found
+// CHECK: Building AST...
+// CHECK: Testing features at each token
+// CHECK: tweak: ExpandAutoType ==> FAIL
+// CHECK: All checks completed, 2 errors
+
+#include "missing.h"
+auto x = []{};

diff  --git a/clang-tools-extra/clangd/test/check.test 
b/clang-tools-extra/clangd/test/check.test
new file mode 100644
index ..832629ce29ef
--- /dev/null
+++ b/clang-tools-extra/clangd/test/check.test
@@ -0,0 +1,13 @@
+# RUN: clangd -log=verbose -check 2>&1 | FileCheck -strict-whitespace %s
+
+CHECK: Testing on source file {{.*}}test.cc
+CHECK: internal (cc1) args are: -cc1
+CHECK: Building preamble...
+CHECK: Built preamble
+CHECK: Building AST...
+CHECK: Testing features at each token
+CHECK-DAG: hover: false
+CHECK-DAG: hover: true
+CHECK-DAG: tweak: AddUsing
+CHECK: All checks completed, 0 errors
+

diff  --git a/clang-tools-extra/clangd/tool/CMakeLists.txt 
b/clang-tools-extra/clangd/tool/CMakeLists.txt
index 670e5a17013a..65e0aa35f265 100644
--- a/clang-tools-extra/clangd/tool/CMakeLists.txt
+++ b/clang-tools-extra/clangd/tool/CMakeLists.txt
@@ -3,6 +3,7 @@ include_directories(${CMAKE_CURRENT_BINARY_DIR}/..)
 
 add_clang_tool(clangd
   ClangdMain.cpp
+  Check.cpp
   $
   )
 

diff  --git a/clang-tools-extra/clangd/tool/Check.cpp 
b/clang-tools-extra/clangd/tool/Check.cpp
new file mode 100644
index ..14ee0fdec9c9
--- /dev/null
+++ b/clang-tools-extra/clangd/tool/Check.cpp
@@ -0,0 +1,258 @@
+//===--- Check.cpp - clangd self-diagnostics 
--===//
+//
+// 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
+//
+//===--===//
+//
+// Many basic problems can occur processing a file in clangd, e.g.:
+//  - system includes are not found
+//  - crash when indexing its AST
+// clangd --check provides a simplified, isolated way to reproduce these,
+// with no editor, LSP, threads, background indexing etc to contend with.
+//
+// One important use case is gathering information for bug reports.
+// Another is reproducing crashes, and checking which setting prevent them.
+//
+// It simulates opening a file (determining compile command, parsing, indexing)
+// and then running features at many locations.
+//
+// Currently it adds some basic logging of progress and results.
+// We should consider extending it to also recognize common symptoms and
+// recommend solutions (e.g. standard library installation issues).
+//
+//===--===//
+
+#include "ClangdLSPServer.h"
+#include "CodeComplete.h"
+#include "GlobalCompilationDatabase.h"
+#include "Hover.h"
+#include "ParsedAST.h"
+#include "Preamble.h"

[PATCH] D88314: Added llvm-string-referencing check

[Bogdan Serea via Phabricator via cfe-commits]

bogser01 updated this revision to Diff 295553.
bogser01 added a comment.

Fixed failing unit test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88314

Files:
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/llvm/StringReferencingCheck.cpp
  clang-tools-extra/clang-tidy/llvm/StringReferencingCheck.h
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-string-referencing.rst
  clang-tools-extra/test/clang-tidy/checkers/llvm-string-referencing.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/llvm-string-referencing.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvm-string-referencing.cpp
@@ -0,0 +1,66 @@
+// RUN: %check_clang_tidy %s llvm-string-referencing %t
+
+namespace std {
+class string {};
+class u18_string_t;
+
+} // namespace std
+
+namespace llvm {
+class StringRef;
+} // namespace llvm
+
+class String;
+
+namespace A {
+using namespace std;
+void f(const string );
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing]
+// CHECK-FIXES: void f(llvm::StringRef P);{{$}}
+} // namespace A
+
+namespace B {
+using std::string;
+void f1(const string );
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing]
+// CHECK-FIXES: void f1(llvm::StringRef P);{{$}}
+} // namespace B
+
+void f2(std::string, int, const std::string &);
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing]
+// CHECK-FIXES: void f2(std::string, int, llvm::StringRef );{{$}}
+void f2(std::string P, int x, const std::string ) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing]
+  // CHECK-FIXES: void f2(std::string P, int x, llvm::StringRef P2) {{{$}}
+  return;
+}
+
+void f3(const std::string , const std::string );
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing]
+// CHECK-MESSAGES: :[[@LINE-2]]:32: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing]
+// CHECK-FIXES: void f3(llvm::StringRef P1, llvm::StringRef P2);{{$}}
+
+struct St {
+  void operator=(const std::string ) const {
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing]
+// CHECK-FIXES: void operator=(llvm::StringRef Val) const {{{$}}
+return;
+  }
+};
+
+void f7(const std::string &);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing]
+// CHECK-FIXES: void f7(llvm::StringRef );{{$}}
+
+// Functions below this line should not trigger the check
+void f1(std::string );
+
+void f4(std::string *P);
+
+void f5(String );
+
+void f6(llvm::StringRef P);
+
+void f9(std::u18_string_t );
+
+void f10(const std::string &);
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-string-referencing.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvm-string-referencing.rst
@@ -0,0 +1,6 @@
+.. title:: clang-tidy - llvm-string-referencing
+
+llvm-string-referencing
+===
+
+FIXME: Describe what patterns does the check detect and why. Give examples.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -192,6 +192,7 @@
`llvm-namespace-comment `_,
`llvm-prefer-isa-or-dyn-cast-in-conditionals `_, "Yes"
`llvm-prefer-register-over-unsigned `_, "Yes"
+   `llvm-string-referencing `_, "Yes"
`llvm-twine-local `_, "Yes"
`llvmlibc-callee-namespace `_,
`llvmlibc-implementation-in-namespace `_,
Index: clang-tools-extra/clang-tidy/llvm/StringReferencingCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/llvm/StringReferencingCheck.h
@@ -0,0 +1,46 @@
+//===--- StringReferencingCheck.h - clang-tidy --*- C++ -*-===//
+//
+// 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
+//

[PATCH] D88634: [clangd] Extend the rename API.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:408
   return CB(InpAST.takeError());
-auto  = InpAST->AST;
-const auto  = AST.getSourceManager();
-auto Loc = sourceLocationInMainFile(SM, Pos);
-if (!Loc)
-  return CB(Loc.takeError());
-const auto *TouchingIdentifier =
-spelledIdentifierTouching(*Loc, AST.getTokens());
-if (!TouchingIdentifier)
-  return CB(llvm::None); // no rename on non-identifiers.
-
-auto Range = halfOpenToRange(
-SM, CharSourceRange::getCharRange(TouchingIdentifier->location(),
-  TouchingIdentifier->endLocation()));
-
-if (RenameOpts.AllowCrossFile)
-  // FIXME: we now assume cross-file rename always succeeds, revisit this.
-  return CB(Range);
-
-// Performing the local rename isn't substantially more expensive than
-// doing an AST-based check, so we just rename and throw away the results.
-auto Changes = clangd::rename({Pos, "dummy", AST, File, Index, RenameOpts,
-   /*GetDirtyBuffer=*/nullptr});
-if (!Changes) {
+clangd::FileIndex EmptyIndex;
+// prepareRename is latency-sensitive:

the empty index is weird here - can we pass nullptr?
Currently nullptr leads to an error in the !crossfile case, but I think we can 
give rename(Index=nullptr, RenameOpts.CrossFile=true) the behavior we want.

(otherwise, if we need an empty index use MemIndex())



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:411
+//  - for single-file rename, performing rename isn't substantially more
+//expensive than doing an AST-based check;
+//  - for cross-file rename, we deliberately pass an empty index to save 
the

mention "the index is used to see if the local rename is complete"?



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:416
+auto Results = clangd::rename(
+{Pos, "dummy", InpAST->AST, File,
+ RenameOpts.AllowCrossFile ?  : Index, RenameOpts});

we're now returning the "dummy" string to the caller, so we should document it 
somewhere (or ideally just make it the empty string and document that)



Comment at: clang-tools-extra/clangd/ClangdServer.h:277
+  ///
+  /// The returned result may be incomplete as it only contains occurrences 
from
+  /// the current main file.

nit: drop "may be incomplete as it"?



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:500
   if (!Opts.AllowCrossFile || RenameDecl.getParentFunctionOrMethod()) {
-return FileEdits(
-{std::make_pair(RInputs.MainFilePath,
-Edit{MainFileCode, std::move(*MainFileRenameEdit)})});
+return RenameResults{
+CurrentIdentifier,

nit: I'd find this more readable as a default construction followed by 
assignments to the fields



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:507
 
-  FileEdits Results;
+  FileEdits Edits;
   // Renameable safely guards us that at this point we are renaming a local

and again here - declare the whole struct first and fill it in?



Comment at: clang-tools-extra/clangd/refactor/Rename.h:58
 
+struct RenameResults {
+  // The range of the symbol that the user can attempt to rename.

nit: I'd suggest RenameResult singular, consistent with CodeCompleteResult and 
ReferencesResult



Comment at: clang-tools-extra/clangd/refactor/Rename.h:60
+  // The range of the symbol that the user can attempt to rename.
+  Range R;
+  FileEdits Edits;

Give this a real name... Target?



Comment at: clang-tools-extra/clangd/refactor/Rename.h:61
+  Range R;
+  FileEdits Edits;
+};

It's slightly awkward to have the half-populated state (may or may not contain 
cross-file results, can't tell without the query).

I'd consider redundantly including `Edit LocalChanges` and `FileEdits 
GlobalChanges` with the former always set and the latter empty when returned 
from preparerename.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88634

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

[PATCH] D88613: [flang] Semantic analysis for FINAL subroutines

[Peter Klausler via Phabricator via cfe-commits]

klausler updated this revision to Diff 295423.
klausler added a comment.

Previous update to this review had inadvertent changes to other files because I 
neglected to rebase after updating master; now fixed.  Sorry for the scare!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88613

Files:
  flang/include/flang/Evaluate/characteristics.h
  flang/include/flang/Evaluate/type.h
  flang/include/flang/Semantics/symbol.h
  flang/include/flang/Semantics/tools.h
  flang/lib/Evaluate/characteristics.cpp
  flang/lib/Evaluate/tools.cpp
  flang/lib/Evaluate/type.cpp
  flang/lib/Semantics/check-call.cpp
  flang/lib/Semantics/check-declarations.cpp
  flang/lib/Semantics/mod-file.cpp
  flang/lib/Semantics/mod-file.h
  flang/lib/Semantics/pointer-assignment.cpp
  flang/lib/Semantics/resolve-names.cpp
  flang/lib/Semantics/symbol.cpp
  flang/lib/Semantics/tools.cpp
  flang/test/Semantics/call03.f90
  flang/test/Semantics/call05.f90
  flang/test/Semantics/final01.f90
  flang/test/Semantics/modfile10.f90
  flang/test/Semantics/resolve32.f90
  flang/test/Semantics/resolve55.f90

Index: flang/test/Semantics/resolve55.f90
===
--- flang/test/Semantics/resolve55.f90
+++ flang/test/Semantics/resolve55.f90
@@ -36,25 +36,24 @@
   end do
 end subroutine s4
 
-subroutine s5()
+module m
 ! Cannot have a variable of a finalizable type in a locality spec
   type t1
 integer :: i
   contains
 final :: f
   end type t1
-
-  type(t1) :: var
-
-!ERROR: Finalizable variable 'var' not allowed in a locality-spec
-  do concurrent(i=1:5) local(var)
-  end do
-
-contains
+ contains
+  subroutine s5()
+type(t1) :: var
+!ERROR: Finalizable variable 'var' not allowed in a locality-spec
+do concurrent(i=1:5) local(var)
+end do
+  end subroutine s5
   subroutine f(x)
 type(t1) :: x
   end subroutine f
-end subroutine s5
+end module m
 
 subroutine s6
 ! Cannot have a nonpointer polymorphic dummy argument in a locality spec
Index: flang/test/Semantics/resolve32.f90
===
--- flang/test/Semantics/resolve32.f90
+++ flang/test/Semantics/resolve32.f90
@@ -57,7 +57,7 @@
   contains
 procedure, nopass :: b => s
 final :: f
-!ERROR: Type parameter, component, or procedure binding 'i' already defined in this type
+!ERROR: FINAL subroutine 'i' of derived type 't2' must be a module procedure
 final :: i
   end type
   type t3
Index: flang/test/Semantics/modfile10.f90
===
--- flang/test/Semantics/modfile10.f90
+++ flang/test/Semantics/modfile10.f90
@@ -64,8 +64,8 @@
 !  type::t2
 !integer(4)::x
 !  contains
-!final::c
 !procedure,non_overridable,private::d
+!final::c
 !  end type
 !  type,abstract::t2a
 !  contains
Index: flang/test/Semantics/final01.f90
===
--- /dev/null
+++ flang/test/Semantics/final01.f90
@@ -0,0 +1,119 @@
+! RUN: %S/test_errors.sh %s %t %f18
+! Test FINAL subroutine constraints C786-C789
+module m1
+  external :: external
+  intrinsic :: sin
+  real :: object
+  procedure(valid), pointer :: pointer
+  type :: parent(kind1, len1)
+integer, kind :: kind1 = 1
+integer, len :: len1 = 1
+  end type
+  type, extends(parent) :: child(kind2, len2)
+integer, kind :: kind2 = 2
+integer, len :: len2 = 2
+   contains
+final :: valid
+!ERROR: FINAL subroutine 'external' of derived type 'child' must be a module procedure
+!ERROR: FINAL subroutine 'sin' of derived type 'child' must be a module procedure
+!ERROR: FINAL subroutine 'object' of derived type 'child' must be a module procedure
+!ERROR: FINAL subroutine 'pointer' of derived type 'child' must be a module procedure
+!ERROR: FINAL subroutine 'func' of derived type 'child' must be a subroutine
+final :: external, sin, object, pointer, func
+!ERROR: FINAL subroutine 's01' of derived type 'child' must have a single dummy argument that is a data object
+!ERROR: FINAL subroutine 's02' of derived type 'child' must have a single dummy argument that is a data object
+!ERROR: FINAL subroutine 's03' of derived type 'child' must not have a dummy argument with INTENT(OUT)
+!ERROR: FINAL subroutine 's04' of derived type 'child' must not have a dummy argument with the VALUE attribute
+!ERROR: FINAL subroutine 's05' of derived type 'child' must not have a POINTER dummy argument
+!ERROR: FINAL subroutine 's06' of derived type 'child' must not have an ALLOCATABLE dummy argument
+!ERROR: FINAL subroutine 's07' of derived type 'child' must not have a coarray dummy argument
+!ERROR: FINAL subroutine 's08' of derived type 'child' must not have a polymorphic dummy argument
+!ERROR: FINAL subroutine 's09' of derived type 'child' must not have a polymorphic dummy argument
+!ERROR: FINAL subroutine 's10' 

[PATCH] D77062: [analyzer] Improve zero assumption in CStringChecke::assumeZero

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@steakhal

> @ASDenysPetrov Do you still want to rework the API of the `assumeZero`?

This patch more looks like NFC, being just refactored.
Actually I see that if we find and fix the root cause, we can freely refuse 
this patch.
Another thing I see is that this patch will work after a root cause fix as well.
And the last one is that as long a root cause stays unfixed, clang will emit an 
assertion without this patch, at least for my testcase.

So, I can't really evaluate this objectively. What do you think?


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

https://reviews.llvm.org/D77062

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

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

[Balázs Kéri via Phabricator via cfe-commits]

balazske marked 12 inline comments as done.
balazske added inline comments.



Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:95
+  std::deque> CalledFunctions{
+  {HandlerDecl, HandlerExpr}};
+

baloghadamsoftware wrote:
> Do we really need to store `FunctionDecl` in the map? The whole code would be 
> much simpler if you only store the call expression and the retrieve the 
> callee declaration once at the beginning of the loop body. Beside simplicity 
> this would also reduce the memory footprint and surely not increase the 
> execution time.
The code was changed to store only the `CallExpr`. The first item is not a 
function call but the reference to function in the `signal` call, so it needs 
special handling.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87449

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