[PATCH] D92006: Refactoring the attrubute plugin example to fit the new API

[Yafei Liu via Phabricator via cfe-commits]

psionic12 updated this revision to Diff 309814.
psionic12 marked an inline comment as done.
psionic12 added a comment.

Fix grammar


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92006

Files:
  clang/examples/Attribute/Attribute.cpp
  clang/test/Frontend/plugin-attribute.cpp

Index: clang/test/Frontend/plugin-attribute.cpp
===
--- clang/test/Frontend/plugin-attribute.cpp
+++ clang/test/Frontend/plugin-attribute.cpp
@@ -1,25 +1,21 @@
-// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -emit-llvm -S %s -o - 2>&1 | FileCheck %s --check-prefix=ATTRIBUTE
-// RUN: not %clang -fplugin=%llvmshlibdir/Attribute%pluginext -emit-llvm -DBAD_ATTRIBUTE -S %s -o - 2>&1 | FileCheck %s --check-prefix=BADATTRIBUTE
+// RUN: %clang -cc1 -load %llvmshlibdir/Attribute%pluginext -DGOOD_ATTR -fsyntax-only -ast-dump -verify=goodattr %s | FileCheck %s
+// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -DBAD_ATTR -fsyntax-only -Xclang -verify=badattr %s
 // REQUIRES: plugins, examples
+#ifdef GOOD_ATTR
+// goodattr-no-diagnostics
+void fn1a() __attribute__((example)) {}
+[[example]] void fn1b() {}
+[[plugin::example]] void fn1c() {}
+void fn2() __attribute__((example("somestring"))) {}
+// CHECK-COUNT-4: -AnnotateAttr 0x{{[0-9a-z]+}} {{}} "example"
+// CHECK: -StringLiteral 0x{{[0-9a-z]+}} {{}} 'const char [{{[0-9]+}}]' lvalue "somestring"
+#endif
 
-void fn1a() __attribute__((example)) { }
-[[example]] void fn1b() { }
-[[plugin::example]] void fn1c() { }
-void fn2() __attribute__((example("somestring"))) { }
-// ATTRIBUTE: warning: 'example' attribute only applies to functions
-int var1 __attribute__((example("otherstring"))) = 1;
-
-// ATTRIBUTE: [[STR1_VAR:@.+]] = private unnamed_addr constant [10 x i8] c"example()\00"
-// ATTRIBUTE: [[STR2_VAR:@.+]] = private unnamed_addr constant [20 x i8] c"example(somestring)\00"
-// ATTRIBUTE: @llvm.global.annotations = {{.*}}@{{.*}}fn1a{{.*}}[[STR1_VAR]]{{.*}}@{{.*}}fn1b{{.*}}[[STR1_VAR]]{{.*}}@{{.*}}fn1c{{.*}}[[STR1_VAR]]{{.*}}@{{.*}}fn2{{.*}}[[STR2_VAR]]
-
-#ifdef BAD_ATTRIBUTE
+#ifdef BAD_ATTR
+int var1 __attribute__((example("otherstring"))) = 1; // badattr-warning {{'example' attribute only applies to functions}}
 class Example {
-  // BADATTRIBUTE: error: 'example' attribute only allowed at file scope
-  void __attribute__((example)) fn3();
+  void __attribute__((example)) fn3(); // badattr-error {{'example' attribute only allowed at file scope}}
 };
-// BADATTRIBUTE: error: 'example' attribute requires a string
-void fn4() __attribute__((example(123))) { }
-// BADATTRIBUTE: error: 'example' attribute takes no more than 1 argument
-void fn5() __attribute__((example("a","b"))) { }
+void fn4() __attribute__((example(123))) { } // badattr-error {{'example's first argument should be a string literal}}
+void fn5() __attribute__((example("a","b", 3, 4.0))) { } // badattr-error {{'example' attribute only allowed at most three arguments}}
 #endif
Index: clang/examples/Attribute/Attribute.cpp
===
--- clang/examples/Attribute/Attribute.cpp
+++ clang/examples/Attribute/Attribute.cpp
@@ -23,9 +23,10 @@
 
 struct ExampleAttrInfo : public ParsedAttrInfo {
   ExampleAttrInfo() {
-// Can take an optional string argument (the check that the argument
-// actually is a string happens in handleDeclAttribute).
-OptArgs = 1;
+// Can take up to 15 optional arguments, to emulate accepting a variadic
+// number of arguments. This just illustrates how many arguments a
+// `ParsedAttrInfo` can hold, we will not use that much in this example.
+OptArgs = 15;
 // GNU-style __attribute__(("example")) and C++-style [[example]] and
 // [[plugin::example]] supported.
 static constexpr Spelling S[] = {{ParsedAttr::AS_GNU, "example"},
@@ -39,7 +40,7 @@
 // This attribute appertains to functions only.
 if (!isa(D)) {
   S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type_str)
-<< Attr << "functions";
+  << Attr << "functions";
   return false;
 }
 return true;
@@ -55,23 +56,33 @@
   S.Diag(Attr.getLoc(), ID);
   return AttributeNotApplied;
 }
-// Check if we have an optional string argument.
-StringRef Str = "";
+// We make some rules here:
+// 1. Only accept at most 3 arguments here.
+// 2. The first argument must be a string literal if it exists.
+if (Attr.getNumArgs() > 3) {
+  unsigned ID = S.getDiagnostics().getCustomDiagID(
+  DiagnosticsEngine::Error,
+  "'example' attribute only allowed at most three arguments");
+  S.Diag(Attr.getLoc(), ID);
+  return AttributeNotApplied;
+}
+// If there are arguments, the first argument should be a string literal.
+Expr *Arg0 = nullptr;
 if (Attr.getNumArgs() > 0) {
-  Expr *ArgExpr = 

[PATCH] D92006: Refactoring the attrubute plugin example to fit the new API

[Yafei Liu via Phabricator via cfe-commits]

psionic12 updated this revision to Diff 309813.
psionic12 added a comment.

Add tests to check if attributes are attached to AST


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92006

Files:
  clang/examples/Attribute/Attribute.cpp
  clang/test/Frontend/plugin-attribute.cpp

Index: clang/test/Frontend/plugin-attribute.cpp
===
--- clang/test/Frontend/plugin-attribute.cpp
+++ clang/test/Frontend/plugin-attribute.cpp
@@ -1,25 +1,21 @@
-// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -emit-llvm -S %s -o - 2>&1 | FileCheck %s --check-prefix=ATTRIBUTE
-// RUN: not %clang -fplugin=%llvmshlibdir/Attribute%pluginext -emit-llvm -DBAD_ATTRIBUTE -S %s -o - 2>&1 | FileCheck %s --check-prefix=BADATTRIBUTE
+// RUN: %clang -cc1 -load %llvmshlibdir/Attribute%pluginext -DGOOD_ATTR -fsyntax-only -ast-dump -verify=goodattr %s | FileCheck %s
+// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -DBAD_ATTR -fsyntax-only -Xclang -verify=badattr %s
 // REQUIRES: plugins, examples
+#ifdef GOOD_ATTR
+// goodattr-no-diagnostics
+void fn1a() __attribute__((example)) {}
+[[example]] void fn1b() {}
+[[plugin::example]] void fn1c() {}
+void fn2() __attribute__((example("somestring"))) {}
+// CHECK-COUNT-4: -AnnotateAttr 0x{{[0-9a-z]+}} {{}} "example"
+// CHECK: -StringLiteral 0x{{[0-9a-z]+}} {{}} 'const char [{{[0-9]+}}]' lvalue "somestring"
+#endif
 
-void fn1a() __attribute__((example)) { }
-[[example]] void fn1b() { }
-[[plugin::example]] void fn1c() { }
-void fn2() __attribute__((example("somestring"))) { }
-// ATTRIBUTE: warning: 'example' attribute only applies to functions
-int var1 __attribute__((example("otherstring"))) = 1;
-
-// ATTRIBUTE: [[STR1_VAR:@.+]] = private unnamed_addr constant [10 x i8] c"example()\00"
-// ATTRIBUTE: [[STR2_VAR:@.+]] = private unnamed_addr constant [20 x i8] c"example(somestring)\00"
-// ATTRIBUTE: @llvm.global.annotations = {{.*}}@{{.*}}fn1a{{.*}}[[STR1_VAR]]{{.*}}@{{.*}}fn1b{{.*}}[[STR1_VAR]]{{.*}}@{{.*}}fn1c{{.*}}[[STR1_VAR]]{{.*}}@{{.*}}fn2{{.*}}[[STR2_VAR]]
-
-#ifdef BAD_ATTRIBUTE
+#ifdef BAD_ATTR
+int var1 __attribute__((example("otherstring"))) = 1; // badattr-warning {{'example' attribute only applies to functions}}
 class Example {
-  // BADATTRIBUTE: error: 'example' attribute only allowed at file scope
-  void __attribute__((example)) fn3();
+  void __attribute__((example)) fn3(); // badattr-error {{'example' attribute only allowed at file scope}}
 };
-// BADATTRIBUTE: error: 'example' attribute requires a string
-void fn4() __attribute__((example(123))) { }
-// BADATTRIBUTE: error: 'example' attribute takes no more than 1 argument
-void fn5() __attribute__((example("a","b"))) { }
+void fn4() __attribute__((example(123))) { } // badattr-error {{'example's first argument should be a string literal}}
+void fn5() __attribute__((example("a","b", 3, 4.0))) { } // badattr-error {{'example' attribute only allowed at most three arguments}}
 #endif
Index: clang/examples/Attribute/Attribute.cpp
===
--- clang/examples/Attribute/Attribute.cpp
+++ clang/examples/Attribute/Attribute.cpp
@@ -23,9 +23,10 @@
 
 struct ExampleAttrInfo : public ParsedAttrInfo {
   ExampleAttrInfo() {
-// Can take an optional string argument (the check that the argument
-// actually is a string happens in handleDeclAttribute).
-OptArgs = 1;
+// Can take up to 15 optional arguments, to emulate accepting a variadic
+// number of arguments. This just illustrates how many arguments a
+// `ParsedAttrInfo` can hold, we will not use that much in this example.
+OptArgs = 15;
 // GNU-style __attribute__(("example")) and C++-style [[example]] and
 // [[plugin::example]] supported.
 static constexpr Spelling S[] = {{ParsedAttr::AS_GNU, "example"},
@@ -39,7 +40,7 @@
 // This attribute appertains to functions only.
 if (!isa(D)) {
   S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type_str)
-<< Attr << "functions";
+  << Attr << "functions";
   return false;
 }
 return true;
@@ -55,23 +56,33 @@
   S.Diag(Attr.getLoc(), ID);
   return AttributeNotApplied;
 }
-// Check if we have an optional string argument.
-StringRef Str = "";
+// We make some rules here:
+// 1. Only accept at most 3 arguments here.
+// 2. The first argument must be a string literal if it exists.
+if (Attr.getNumArgs() > 3) {
+  unsigned ID = S.getDiagnostics().getCustomDiagID(
+  DiagnosticsEngine::Error,
+  "'example' attribute only allowed at most three arguments");
+  S.Diag(Attr.getLoc(), ID);
+  return AttributeNotApplied;
+}
+// If has arguments, the first argument should be a string literal.
+Expr *Arg0 = nullptr;
 if (Attr.getNumArgs() > 0) {
-  Expr *ArgExpr = Attr.getA

[PATCH] D92742: [clang] Add support for attribute 'swift_async'

[Doug Gregor via Phabricator via cfe-commits]

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

This looks good. I went ahead and tested it as part of adoption in Swift's 
Clang importer, over at https://github.com/apple/swift/pull/34985


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

https://reviews.llvm.org/D92742

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

[PATCH] D90392: [clang-tidy] Omit std::make_unique/make_shared for default initialization.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

I think the patches description still could use some more words,
in particular it still only repeats what the patch does,
but not why it does that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90392

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

[PATCH] D91821: Fix PR42049 - Crash when parsing bad decltype use within template argument list after name assumed to be a function template

[Faisal Vali via Phabricator via cfe-commits]

faisalv updated this revision to Diff 309809.
faisalv edited the summary of this revision.
faisalv added a comment.

Per Richard's suggestion, instead of including the cached tokens into the 
decltype annotation, i revert the cache to match the end of where we think the 
(broken) decltype annotated token should end.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91821

Files:
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/Parser/PR42049.cpp


Index: clang/test/Parser/PR42049.cpp
===
--- /dev/null
+++ clang/test/Parser/PR42049.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// https://bugs.llvm.org/show_bug.cgi?id=42049
+
+void f() {
+  g(); // expected-error  {{use of undeclared identifier}} 
expected-error {{expected}}
+  g2 void f2() {
+  g(); // expected-error {{use of undeclared identifier}} 
expected-error {{expected}}
+  g2Index: clang/test/Parser/PR42049.cpp
===
--- /dev/null
+++ clang/test/Parser/PR42049.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// https://bugs.llvm.org/show_bug.cgi?id=42049
+
+void f() {
+  g(); // expected-error  {{use of undeclared identifier}} expected-error {{expected}}
+  g2 void f2() {
+  g(); // expected-error {{use of undeclared identifier}} expected-error {{expected}}
+  g2___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 5755522 - Sema.h: delete unused variables/functions/type aliases

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2020-12-06T20:39:01-08:00
New Revision: 5755522b5a8be69d8a9f3b05c5e7078298d0d581

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

LOG: Sema.h: delete unused variables/functions/type aliases

Added: 


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

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index c2fc5dface7f..4eddd07f66d0 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1072,10 +1072,6 @@ class Sema final {
   /// have been declared.
   bool GlobalNewDeleteDeclared;
 
-  /// A flag to indicate that we're in a context that permits abstract
-  /// references to fields.  This is really a
-  bool AllowAbstractFieldReference;
-
   /// Describes how the expressions currently being parsed are
   /// evaluated at run-time, if at all.
   enum class ExpressionEvaluationContext {
@@ -1134,9 +1130,6 @@ class Sema final {
 /// Whether the enclosing context needed a cleanup.
 CleanupInfo ParentCleanup;
 
-/// Whether we are in a decltype expression.
-bool IsDecltype;
-
 /// The number of active cleanup objects when we entered
 /// this expression evaluation context.
 unsigned NumCleanupObjects;
@@ -1635,7 +1628,6 @@ class Sema final {
 llvm::Optional ImmediateDiag;
 llvm::Optional PartialDiagId;
   };
-  using DiagBuilderT = SemaDiagnosticBuilder;
 
   /// Is the last error level diagnostic immediate. This is used to determined
   /// whether the next info diagnostic should be immediate.
@@ -2810,12 +2802,6 @@ class Sema final {
   /// and partial specializations are visible, and diagnose if not.
   void checkSpecializationVisibility(SourceLocation Loc, NamedDecl *Spec);
 
-  /// We've found a use of a template specialization that would select a
-  /// partial specialization. Check that the partial specialization is visible,
-  /// and diagnose if not.
-  void checkPartialSpecializationVisibility(SourceLocation Loc,
-NamedDecl *Spec);
-
   /// Retrieve a suitable printing policy for diagnostics.
   PrintingPolicy getPrintingPolicy() const {
 return getPrintingPolicy(Context, PP);

diff  --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 5710f9e3daad..a465c6594851 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -4,14 +4,3 @@ void 
Sema::checkSpecializationVisibility(SourceLocation Loc, NamedDecl *Spec) {
 
   ExplicitSpecializationVisibilityChecker(*this, Loc).check(Spec);
 }
-
-/// Check whether a template partial specialization that we've discovered
-/// is hidden, and produce suitable diagnostics if so.
-void Sema::checkPartialSpecializationVisibility(SourceLocation Loc,
-NamedDecl *Spec) {
-  llvm::SmallVector Modules;
-  if (!hasVisibleDeclaration(Spec, &Modules))
-diagnoseMissingImport(Loc, Spec, Spec->getLocation(), Modules,
-  MissingImportKind::PartialSpecialization,
-  /*Recover*/true);
-}



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

[clang] 192fb1b - [Sema] Delete unused declarations

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2020-12-06T20:16:00-08:00
New Revision: 192fb1bd8ac6a59fb2efd528038fd13c53e9ff46

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

LOG: [Sema] Delete unused declarations

Notes about some declarations:

* clang::Sema::endsWithnarrowing: deleted by rC148381
* clang::Sema::ConvertIntegerToTypeWarnOnOverflow: deleted by rC214678
* clang::Sema::FreePackedContext: deleted by rC268085
* clang::Sema::ComputeDefaulted*: deleted by rC296067

Added: 


Modified: 
clang/include/clang/Sema/CodeCompleteConsumer.h
clang/include/clang/Sema/Initialization.h
clang/include/clang/Sema/Sema.h

Removed: 




diff  --git a/clang/include/clang/Sema/CodeCompleteConsumer.h 
b/clang/include/clang/Sema/CodeCompleteConsumer.h
index 7293784f894b..87646ab95025 100644
--- a/clang/include/clang/Sema/CodeCompleteConsumer.h
+++ b/clang/include/clang/Sema/CodeCompleteConsumer.h
@@ -992,9 +992,6 @@ inline bool operator>=(const CodeCompletionResult &X,
   return !(X < Y);
 }
 
-raw_ostream &operator<<(raw_ostream &OS,
-  const CodeCompletionString &CCS);
-
 /// Abstract interface for a consumer of code-completion
 /// information.
 class CodeCompleteConsumer {

diff  --git a/clang/include/clang/Sema/Initialization.h 
b/clang/include/clang/Sema/Initialization.h
index 2245c1505001..8115f69f4df3 100644
--- a/clang/include/clang/Sema/Initialization.h
+++ b/clang/include/clang/Sema/Initialization.h
@@ -1226,17 +1226,6 @@ class InitializationSequence {
   /// constructor.
   bool isConstructorInitialization() const;
 
-  /// Returns whether the last step in this initialization sequence is a
-  /// narrowing conversion, defined by C++0x [dcl.init.list]p7.
-  ///
-  /// If this function returns true, *isInitializerConstant will be set to
-  /// describe whether *Initializer was a constant expression.  If
-  /// *isInitializerConstant is set to true, *ConstantValue will be set to the
-  /// evaluated value of *Initializer.
-  bool endsWithNarrowing(ASTContext &Ctx, const Expr *Initializer,
- bool *isInitializerConstant,
- APValue *ConstantValue) const;
-
   /// Add a new step in the initialization that resolves the address
   /// of an overloaded function to a specific function declaration.
   ///
@@ -1362,10 +1351,6 @@ class InitializationSequence {
   /// from a zero constant.
   void AddOCLZeroOpaqueTypeStep(QualType T);
 
-  /// Add a step to initialize by zero types defined in the
-  /// cl_intel_device_side_avc_motion_estimation OpenCL extension
-  void AddOCLIntelSubgroupAVCZeroInitStep(QualType T);
-
   /// Add steps to unwrap a initializer list for a reference around a
   /// single element and rewrap it at the end.
   void RewrapReferenceInitList(QualType T, InitListExpr *Syntactic);

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 980be69cb1a5..c2fc5dface7f 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -455,11 +455,7 @@ class Sema final {
 std::string SectionName;
 bool Valid = false;
 SourceLocation PragmaLocation;
-
-void Act(SourceLocation PragmaLocation,
- PragmaClangSectionAction Action,
- StringLiteral* Name);
-   };
+  };
 
PragmaClangSection PragmaClangBSSSection;
PragmaClangSection PragmaClangDataSection;
@@ -5713,45 +5709,6 @@ class Sema final {
 }
   };
 
-  /// Determine what sort of exception specification a defaulted
-  /// copy constructor of a class will have.
-  ImplicitExceptionSpecification
-  ComputeDefaultedDefaultCtorExceptionSpec(SourceLocation Loc,
-   CXXMethodDecl *MD);
-
-  /// Determine what sort of exception specification a defaulted
-  /// default constructor of a class will have, and whether the parameter
-  /// will be const.
-  ImplicitExceptionSpecification
-  ComputeDefaultedCopyCtorExceptionSpec(CXXMethodDecl *MD);
-
-  /// Determine what sort of exception specification a defaulted
-  /// copy assignment operator of a class will have, and whether the
-  /// parameter will be const.
-  ImplicitExceptionSpecification
-  ComputeDefaultedCopyAssignmentExceptionSpec(CXXMethodDecl *MD);
-
-  /// Determine what sort of exception specification a defaulted move
-  /// constructor of a class will have.
-  ImplicitExceptionSpecification
-  ComputeDefaultedMoveCtorExceptionSpec(CXXMethodDecl *MD);
-
-  /// Determine what sort of exception specification a defaulted move
-  /// assignment operator of a class will have.
-  ImplicitExceptionSpecification
-  ComputeDefaultedMoveAssignmentExceptionSpec(CXXMethodDecl *MD);
-
-  /// Determine what sort of exception specification a defaulted
-  /// destructor of a class w

[PATCH] D92742: [clang] Add support for attribute 'swift_async'

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added reviewers: arphaman, doug.gregor, aaron.ballman.
Herald added subscribers: jdoerfert, ributzka, jkorous.
erik.pilkington requested review of this revision.

This attributes specifies how (or if) a given function or method will be 
imported into a swift async method. You can read more about swift async here: 
https://github.com/DougGregor/swift-evolution/blob/concurrency-objc/proposals/-concurrency-objc.md.
 rdar://70111252

Thanks for taking a look!


https://reviews.llvm.org/D92742

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/attr-swift-async.m

Index: clang/test/SemaObjC/attr-swift-async.m
===
--- /dev/null
+++ clang/test/SemaObjC/attr-swift-async.m
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fblocks %s
+// RUN: %clang_cc1 -xobjective-c++ -verify -fsyntax-only -fblocks %s
+
+#define SA(...) __attribute__((swift_async(__VA_ARGS__)))
+
+SA(none) int a; // expected-warning{{'swift_async' attribute only applies to functions and Objective-C methods}}
+
+SA(none) void b();
+
+SA(not_swift_private, 0) void c(); // expected-error{{'swift_async' attribute parameter 2 is out of bounds}}
+SA(swift_private, 1) void d(); // expected-error{{'swift_async' attribute parameter 2 is out of bounds}}
+SA(swift_private, 1) void e(int); // expected-error{{'swift_async' completion handler parameter must have block type returning 'void', type here is 'int'}}
+SA(not_swift_private, 1) void f(int (^)()); // expected-error{{'swift_async' completion handler parameter must have block type returning 'void', type here is 'int (^)()'}}
+SA(swift_private, 1) void g(void (^)());
+
+SA(none, 1) void h(); // expected-error{{'swift_async' attribute takes one argument}}
+SA() void i(); // expected-error{{'swift_async' attribute takes at least 1 argument}}
+SA(not_swift_private) void j(); // expected-error{{'swift_async' attribute requires exactly 2 arguments}}
+SA(43) void k(); // expected-error{{'swift_async' attribute requires parameter 1 to be an identifier}}
+SA(not_a_thing, 0) void l(); // expected-error{{first argument to 'swift_async' must be either 'none', 'swift_private', or 'not_swift_private'}}
+
+@interface TestOnMethods
+-(void)m1:(int (^)())callback SA(swift_private, 1); // expected-error{{'swift_async' completion handler parameter must have block type returning 'void', type here is 'int (^)()'}}
+-(void)m2:(void (^)())callback SA(swift_private, 0); // expected-error{{'swift_async' attribute parameter 2 is out of bounds}}
+-(void)m3:(void (^)())callback SA(swift_private, 2); // expected-error{{'swift_async' attribute parameter 2 is out of bounds}}
+-(void)m4 SA(none);
+-(void)m5:(int)p handler:(void (^)(int))callback SA(not_swift_private, 2);
+@end
+
+#ifdef __cplusplus
+struct S {
+  SA(none) void mf1();
+  SA(swift_private, 2) void mf2(void (^)());
+  SA(swift_private, 1) void mf3(void (^)()); // expected-error{{'swift_async' attribute is invalid for the implicit this argument}}
+  SA(swift_private, 0) void mf4(void (^)()); // expected-error{{'swift_async' attribute parameter 2 is out of bounds}}
+  SA(not_swift_private, 2) void mf5(int (^)()); // expected-error{{'swift_async' completion handler parameter must have block type returning 'void', type here is 'int (^)()'}}
+};
+#endif
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
@@ -147,6 +147,7 @@
 // CHECK-NEXT: Section (SubjectMatchRule_function, SubjectMatchRule_variable_is_global, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property)
 // CHECK-NEXT: SetTypestate (SubjectMatchRule_function_is_member)
 // CHECK-NEXT: SpeculativeLoadHardening (SubjectMatchRule_function, SubjectMatchRule_objc_method)
+// CHECK-NEXT: SwiftAsync (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: SwiftBridgedTypedef (SubjectMatchRule_type_alias)
 // CHECK-NEXT: SwiftContext (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: SwiftError (SubjectMatchRule_function, SubjectMatchRule_objc_method)
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6033,6 +6033,56 @@
   D->addAttr(::new (S.Context) SwiftNewTypeAttr(S.Context, AL, Kind));
 }
 
+static void handleSwiftAsyncAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  if (!AL.isArgIdent(0)) {
+S.Diag(AL.getLoc(), diag::err_attribute_argument_n_type)
+<< AL << 1 << AANT_ArgumentIdenti

[PATCH] D90392: [clang-tidy] Omit std::make_unique/make_shared for default initialization.

[Chris Kennelly via Phabricator via cfe-commits]

ckennelly added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90392

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

LLVM buildmaster will be updated and restarted tonight

[Galina Kistanova via cfe-commits]

 Hello everyone,

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

Thanks

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

[PATCH] D92329: [PowerPC][Clang] Remove QPX support

[Qing Shan Zhang via Phabricator via cfe-commits]

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

LGTM as I grep the whole repo with this patch applied, no QPX any more.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92329

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

[PATCH] D91975: [clang-tidy] cppcoreguidelines Narrowing Conversions Check: detect narrowing conversions involving typedefs

[Eric Seidel via Phabricator via cfe-commits]

gridaphobe marked 2 inline comments as done.
gridaphobe added a comment.
Herald added a subscriber: shchenz.

I've added the non-narrowing tests, are we good to merge?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91975

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

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

In D87974#2432793 , @zoecarver wrote:

> So, it looks like GCC already uses `__builtin_clear_padding` and MSVC already 
> uses `__builtin_zero_non_value_bits`. This patch (obviously) is currently 
> implementing `__builtin_zero_non_value_bits ` but, I had planned to update it 
> to use `__builtin_clear_padding`. Maybe that's not the best course of action, 
> though.
>
> We should all try to agree on _one_ name. CC @BillyONeal @jwakely thoughts?

The name MSFT is already shipping in production is 
`__builtin_zero_non_value_bits`. If gcc is already shipping another name in 
production I think clang is stuck supporting both names, if gcc has not yet 
shipped their implementation perhaps we can choose one. That seems to be more 
on gcc than it is on clang given clang's desire to be more or less a drop in 
replacement for either gcc or msvc.

The MSFT STL implementation can of course use a different builtin when we 
detect clang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87974

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

[clang] a2f9221 - [TableGen] Delete 11 unused declarations

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2020-12-06T13:21:07-08:00
New Revision: a2f922140f5380571fb74179f2bf622b3b925697

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

LOG: [TableGen] Delete 11 unused declarations

Added: 


Modified: 
clang/utils/TableGen/NeonEmitter.cpp
clang/utils/TableGen/TableGenBackends.h
llvm/include/llvm/TableGen/Record.h
llvm/lib/TableGen/DetailedRecordsBackend.cpp
llvm/lib/TableGen/JSONBackend.cpp
llvm/utils/TableGen/CodeGenDAGPatterns.h
llvm/utils/TableGen/GICombinerEmitter.cpp
llvm/utils/TableGen/GlobalISelEmitter.cpp
llvm/utils/TableGen/SubtargetEmitter.cpp

Removed: 




diff  --git a/clang/utils/TableGen/NeonEmitter.cpp 
b/clang/utils/TableGen/NeonEmitter.cpp
index d5bf59ef04ad..e8340d976f5e 100644
--- a/clang/utils/TableGen/NeonEmitter.cpp
+++ b/clang/utils/TableGen/NeonEmitter.cpp
@@ -580,21 +580,18 @@ class NeonEmitter {
 ClassMap[NoTestOpI] = ClassNoTest;
   }
 
-  // run - Emit arm_neon.h.inc
+  // Emit arm_neon.h.inc
   void run(raw_ostream &o);
 
-  // runFP16 - Emit arm_fp16.h.inc
+  // Emit arm_fp16.h.inc
   void runFP16(raw_ostream &o);
 
-  // runBF16 - Emit arm_bf16.h.inc
+  // Emit arm_bf16.h.inc
   void runBF16(raw_ostream &o);
 
-  // runHeader - Emit all the __builtin prototypes used in arm_neon.h,
-  // arm_fp16.h and arm_bf16.h
+  // Emit all the __builtin prototypes used in arm_neon.h, arm_fp16.h and
+  // arm_bf16.h
   void runHeader(raw_ostream &o);
-
-  // runTests - Emit tests for all the Neon intrinsics.
-  void runTests(raw_ostream &o);
 };
 
 } // end anonymous namespace

diff  --git a/clang/utils/TableGen/TableGenBackends.h 
b/clang/utils/TableGen/TableGenBackends.h
index dc4476cdff44..33a06bfe4469 100644
--- a/clang/utils/TableGen/TableGenBackends.h
+++ b/clang/utils/TableGen/TableGenBackends.h
@@ -93,9 +93,6 @@ void EmitFP16(llvm::RecordKeeper &Records, llvm::raw_ostream 
&OS);
 void EmitBF16(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 void EmitNeonSema(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 void EmitNeonTest(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
-void EmitNeon2(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
-void EmitNeonSema2(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
-void EmitNeonTest2(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 
 void EmitSveHeader(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 void EmitSveBuiltins(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);

diff  --git a/llvm/include/llvm/TableGen/Record.h 
b/llvm/include/llvm/TableGen/Record.h
index 1c3ec5fb21f5..fe552331b385 100644
--- a/llvm/include/llvm/TableGen/Record.h
+++ b/llvm/include/llvm/TableGen/Record.h
@@ -1608,7 +1608,6 @@ class Record {
 return IsAnonymous;
   }
 
-  void print(raw_ostream &OS) const;
   void dump() const;
 
   
//======//

diff  --git a/llvm/lib/TableGen/DetailedRecordsBackend.cpp 
b/llvm/lib/TableGen/DetailedRecordsBackend.cpp
index 1b6b675081ed..2c3c3358b347 100644
--- a/llvm/lib/TableGen/DetailedRecordsBackend.cpp
+++ b/llvm/lib/TableGen/DetailedRecordsBackend.cpp
@@ -52,7 +52,6 @@ class DetailedRecordsEmitter {
   void printTemplateArgs(Record *Rec, raw_ostream &OS);
   void printSuperclasses(Record *Rec, raw_ostream &OS);
   void printFields(Record *Rec, raw_ostream &OS);
-  std::string formatLocation(const SMLoc Loc);
 }; // emitter class
 
 } // anonymous namespace

diff  --git a/llvm/lib/TableGen/JSONBackend.cpp 
b/llvm/lib/TableGen/JSONBackend.cpp
index ea82934e5d3b..131650f987fb 100644
--- a/llvm/lib/TableGen/JSONBackend.cpp
+++ b/llvm/lib/TableGen/JSONBackend.cpp
@@ -29,7 +29,6 @@ class JSONEmitter {
   RecordKeeper &Records;
 
   json::Value translateInit(const Init &I);
-  json::Array listSuperclasses(const Record &R);
 
 public:
   JSONEmitter(RecordKeeper &R);

diff  --git a/llvm/utils/TableGen/CodeGenDAGPatterns.h 
b/llvm/utils/TableGen/CodeGenDAGPatterns.h
index 7a0da531469e..c0c45a74de66 100644
--- a/llvm/utils/TableGen/CodeGenDAGPatterns.h
+++ b/llvm/utils/TableGen/CodeGenDAGPatterns.h
@@ -436,8 +436,6 @@ class ScopedName {
   unsigned getScope() const { return Scope; }
   const std::string &getIdentifier() const { return Identifier; }
 
-  std::string getFullName() const;
-
   bool operator==(const ScopedName &o) const;
   bool operator!=(const ScopedName &o) const;
 };

diff  --git a/llvm/utils/TableGen/GICombinerEmitter.cpp 
b/llvm/utils/TableGen/GICombinerEmitter.cpp
index 5f091467636d..c3bbfd28f965 100644
--- a/llvm/utils/TableGen/GICombinerEmitter.cpp
+++ b/llvm/utils/TableGen/GICombinerEmitter.cpp
@@ -615,7 +615,6 @@ class GICombinerEmitter {
   /// response to the generated cl::opt.
   void emitNameMatcher(raw_ostream &OS) c

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91913

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

[PATCH] D90271: [clang][ToolChains] explicitly return LangOptions::StackProtectorMode

[Solomon Choina via Phabricator via cfe-commits]

SolarAquarion added a comment.

You missed a change https://bugs.llvm.org/show_bug.cgi?id=48413


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90271

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

[PATCH] D92733: Fix PR25627 - false positive diagnostics involving implicit-captures in dependent lambda expressions.

[Faisal Vali via Phabricator via cfe-commits]

faisalv created this revision.
faisalv added reviewers: rsmith, aaron.ballman, wchilders, BRevzin.
faisalv added a project: clang.
faisalv requested review of this revision.

This patch attempts to address the following bugs involving lambda-captures:

1. /https://bugs.llvm.org/show_bug.cgi?id=25627 which emits a false positive 
diagnostic for the following code: ```void f() { constexpr int I = 10; [](auto 
a) { return I; }; };```
  - this occurs because our current logic does not recognize that even though 
our expressions are not instantiation dependent, if that expression is being 
used to initialize some dependent entity, an lvalue-to-rvalue conversion might 
still be introduced during the instantiation, which might avert odr-use.  (we 
do not do return type deduction until instantiation).
  - to address this, I pass in the destination type to ActOnFullExpr, if the 
expression might be used in such an initialization, and use a 
PotentialResultsVisitor to determine the potential results of the 
full-expression, and then use all that information to avoid such false positive 
diagnostics.
2. it fixes any unnecessary odr-uses triggered in discarded-value expressions 
and thus some tests marked as FIXMES.
  - we simply make a call to CheckLValueToRValueConversionOperand() that 
rebuilds the relevant AST marking each potential result as non-odr-used for 
discarded value expressions even in the absence of an lvalue-to-rvalue 
conversion.
3. I do have some questions, regarding the intended behavior:

  [] {
if (false) {
  const int& i = I; <-- This is diagnosed, should it be - even though it is 
never instantiated?
}
  };
  
  [](auto a) {
if (sizeof(a) > 0) {
  const int &i = I; <-- This is diagnosed, should it be - even though it is 
never instantiated?
}
  }
  
  struct X {
constexpr static int s = 10;
int i = 10;
  };
  
  void f() {
constexpr X x;
[] { 
  int i = x.s;  // <-- This is diagnosed, should it be - even though it 
does not really use 'x' since 's' is static
  int j = x.i;  // this does not odr-use x.
}; 
  }
   

Thank you!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92733

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CXX/basic/basic.def.odr/p2.cpp
  clang/test/CXX/drs/dr7xx.cpp
  clang/test/SemaCXX/PR25627-generic-lambdas-capturing.cpp

Index: clang/test/SemaCXX/PR25627-generic-lambdas-capturing.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/PR25627-generic-lambdas-capturing.cpp
@@ -0,0 +1,326 @@
+// RUN: %clang_cc1 -std=c++17 -verify -fsyntax-only -fblocks -emit-llvm-only -Wno-unused-value %s
+// DONTRUNYET: %clang_cc1 -std=c++1y -verify -fsyntax-only -fblocks -fdelayed-template-parsing %s -DDELAYED_TEMPLATE_PARSING
+// DONTRUNYET: %clang_cc1 -std=c++1y -verify -fsyntax-only -fblocks -fms-extensions %s -DMS_EXTENSIONS
+// DONTRUNYET: %clang_cc1 -std=c++1y -verify -fsyntax-only -fblocks -fdelayed-template-parsing -fms-extensions %s -DMS_EXTENSIONS -DDELAYED_TEMPLATE_PARSING
+
+constexpr int ODRUSE_SZ = sizeof(char);
+
+template
+void f(T, const int (&)[N]) { }
+
+template
+void f(const T&, const int (&)[ODRUSE_SZ]) { }
+
+#define DEFINE_SELECTOR(x)   \
+  int selector_ ## x[sizeof(x) == ODRUSE_SZ ? ODRUSE_SZ : ODRUSE_SZ + 5]
+
+#define F_CALL(x, a) f(x, selector_ ## a)
+
+// This is a risky assumption, because if an empty class gets captured by value
+// the lambda's size will still be '1' 
+#define ASSERT_NO_CAPTURES(L) static_assert(sizeof(L) == 1, "size of closure with no captures must be 1")
+#define ASSERT_CLOSURE_SIZE_EXACT(L, N) static_assert(sizeof(L) == (N), "size of closure must be " #N)
+#define ASSERT_CLOSURE_SIZE(L, N) static_assert(sizeof(L) >= (N), "size of closure must be >=" #N)
+
+
+
+namespace sample {
+  struct X {  
+int i;
+constexpr X(int i) : i(i) { }
+  };
+  struct XVal {
+constexpr XVal(X) { }
+constexpr XVal() = default;
+  };
+  struct XRef {
+ constexpr XRef(const X&) { }
+  };
+} 
+
+namespace ns105 {
+
+template V f(V);
+void f(...);
+template void f(T&, T&, T);
+struct X { };
+template auto foo() {
+  extern int ei;
+  extern const U eu;
+  constexpr X x;
+  constexpr U u;
+  constexpr int I = 10;
+  auto L = [](auto a) {
+ return f(I);
+ return f(x,a);
+ return f(x);
+ U& u = eu;
+ const int &ir = ei;
+ return f(u);
+  };
+  return L;
+}
+
+}
+
+
+struct X72 {
+  constexpr static int s = 10;
+  int i;
+  int arr[4] = {1, 2, 3, 4};
+};
+void foo1_72() {
+   constexpr X72 x{5};
+   constexpr X72 y{15};
+   constexpr X72 z{25};
+   constexpr X72 w{25};
+   constexpr int arr[] = { 1, 2, 3 };
+   constexpr auto pMemI = &X72::i;
+   constexpr auto pMemArr = &X72::arr;
+   conste

[PATCH] D92257: [clang-format] Add option to control the space at the front of a line comment

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:2727
+
+  * ``unsigned Maximum`` The maximum number of spaces at the start of the 
comment.
+

HazardyKnusperkeks wrote:
> MyDeveloperDay wrote:
> > I'm personally not a massive fan of stuffing -1 into an unsigned but I 
> > understand why its there, if this was an signed and it was actually -1 
> > would the algorithm be substantially worse in your view?
> I'm no fan if unsigned in any way, but that seems to be the way in 
> clang-format.
> Making it signed would require a few more checks when to use it, but I don't 
> see any problem in that.
> I just would also make the Minimum signed then just to be consistent.
> 
> While parsing the style I would add checks to ensure Minimum is never 
> negative and Maximum is always greater or equal to -1, should that print any 
> warnings? Is there a standard way of doing so? Or should it be just silently 
> corrected?
I find it confusing why we have 2, Minimum and Maximum, instead of a single one.
I'm not convinced that `Maximum` is useful.
Conceptually I'd prefer a single integer option, say `LineCommentContentIndent` 
that would indicate the default indent used for the content of line comments. 
I'd naively expect `LineCommentContentIndent = `:
* 0 would produce `//comment`
* 1 would produce `// comment` (current default)
* 2 would produce `//  comment`, etc.
and this will work with if the input is any of `//comment`, `// comment`, or 
`//  comment`, etc.

An additional consideration is that line comment sections often contain 
additional indentation, e.g. when there is a bullet list, paragraphs, etc. and 
so we can't guarantee that the indent of each line comment will be less than 
Maximum in general. I'd expect this feature to not adjust extra indent in 
comments, e.g.,
```
// Lorem ipsum dolor sit amet,
//  consectetur adipiscing elit,
//  ...
```
after reformatting with `LineCommentContentIndent=0` to produce
```
//Lorem ipsum dolor sit amet,
// consectetur adipiscing elit,
// ...
```
(and vice-versa, after reformatting with `LineCommentContentIndent=1`).
This may well be handled by code, I just wasn't sure by looking at the code and 
test examples.



Comment at: clang/lib/Format/BreakableToken.cpp:790
+  (Style.Language != FormatStyle::LK_TextProto ||
+   OriginalPrefix[i].substr(0, 2) != "##")) {
+Prefix[i] = IndentPrefix.str();

HazardyKnusperkeks wrote:
> HazardyKnusperkeks wrote:
> > MyDeveloperDay wrote:
> > > is this case covered by a unit test at all? sorry can you explain why you 
> > > are looking for "##"?
> > It is covered by multiple tests, that's how I was made aware of it. :)
> > If you look at the code before it only adds a space if the old prefix is 
> > "#" not "##" which is also found by `getLineCommentIndentPrefix`. As it 
> > seems in `TextProto` "##" should not be touched. I can of course add a test 
> > in my test function.
> > 
> > Now I see a change, in the code before "#" was only accepted when the 
> > language is `TextProto`, now it is always. But I think for that to happen 
> > the parser (or lexer?) should have assigned something starting with"#" as 
> > comment, right? But I can change that.
> Okay # # is formatted, I try again:
> If you look at the code before it only adds a space if the old prefix is "#" 
> not "`##`" which is also found by `getLineCommentIndentPrefix`. As it seems 
> in `TextProto` "`##`" should not be touched.
Thanks for the analysis!
I wrote the text proto comment detection. I believe the current clang-format is 
buggy in that it should transform `##comment` into `## comment` for text proto 
(and similarly for all other `KnownTextProtoPrefixes` in 
`getLineCommentIndentCommentPrefix`), so this `substr(0, 2) != "##"` is 
unnecessary and I should go ahead and update and add tests for that.



Comment at: clang/unittests/Format/FormatTestComments.cpp:3405
+"//  Lorem   ipsum\n"
+"//  dolor   sit amet\n" // Why are here the spaces dropped?
+"\n"

This is desired, AFAIK, and due to the normalization behavior while reflowing: 
when a comment line exceeds the comment limit and is broken up into a new line, 
the full range of blanks is replaced with a newline. 
(https://github.com/llvm/llvm-project/blob/ddb002d7c74c038b64dd9d3c3e4a4b58795cf1a6/clang/lib/Format/BreakableToken.cpp#L66).
Note that reflowing copies the extra indent of the line, e.g.,
```
// line limit  V
// heading
// *line is
//   long long long long 
```
get reformatted as
```
// line limit  V
// heading
// *line is
//   long long
//   long long 
```
so if for ranges of blanks longer of size S>1 we copied the (S-1) blanks at the 
beginning of the next line, we would have cascading comment reflows undesired 
with longer and longer indents.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  http

[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

[Nikita Popov via Phabricator via cfe-commits]

nikic abandoned this revision.
nikic added a comment.

This needs someone with access to AArch64 hardware to look into 
https://reviews.llvm.org/D87188#2281093 to make progress. I don't have any 
AArch64 hardware, and judging by the time it takes to build cmake on an AArch64 
machine in the GCC compile farm, there's no way I'm going to be doing LLVM 
builds there.


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

https://reviews.llvm.org/D87188

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

[PATCH] D88220: [C++20] P1825R0: More implicit moves

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

I've found another case that Clang has never handled correctly (and which your 
patch does not fix)—
https://godbolt.org/z/xd8qGW

  struct Widget {};
  struct Frodo {
  Frodo(Widget&);
  Frodo(Widget&&) = delete;
  };
  
  Frodo twelve() {
  Widget w;
  return w;  // according to the majority of vendors, the first pass should 
not "fail";
 // it should successfully find the deleted overload and 
hard-error at that point.
 // This has been the case ever since C++11, AFAIK.
  }

This example will be mentioned in my upcoming (not-yet-finished) WG21 paper 
P2266 , as an example of why the two-pass mechanism 
sucks and should be removed from C++2b.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88220

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

[PATCH] D68410: [AttrDocs] document always_inline

[Miguel Ojeda via Phabricator via cfe-commits]

ojeda added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:5498
+
+See also `the MSDN Inline docs`_, `the GCC Common Function Attribute docs`_,
+and `the GCC Inline docs`_.

MSDN is gone now, Microsoft Docs is the new one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68410

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

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

[Douglas Chen via Phabricator via cfe-commits]

dougpuob updated this revision to Diff 309767.
dougpuob added a comment.

- Improved code review suggestions from @njames93. Including move the 
IdentifierNamingCheck::HNOption variable to 
IdentifierNamingCheck::FileStyle::HNOption, use try_emplace() instead of 
insert() and lookup().


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h

Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
===
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
@@ -84,19 +84,26 @@
   struct FileStyle {
 FileStyle() : IsActive(false), IgnoreMainLikeFunctions(false) {}
 FileStyle(SmallVectorImpl> &&Styles,
-  bool IgnoreMainLike)
-: Styles(std::move(Styles)), IsActive(true),
-  IgnoreMainLikeFunctions(IgnoreMainLike) {}
+  HungarianNotationOption HNOption, bool IgnoreMainLike)
+: Styles(std::move(Styles)), HNOption(std::move(HNOption)),
+  IsActive(true), IgnoreMainLikeFunctions(IgnoreMainLike) {}
 
 ArrayRef> getStyles() const {
   assert(IsActive);
   return Styles;
 }
+
+const HungarianNotationOption &getHNOption() const {
+  assert(IsActive);
+  return HNOption;
+}
+
 bool isActive() const { return IsActive; }
 bool isIgnoringMainLikeFunction() const { return IgnoreMainLikeFunctions; }
 
   private:
 SmallVector, 0> Styles;
+HungarianNotationOption HNOption;
 bool IsActive;
 bool IgnoreMainLikeFunctions;
   };
@@ -121,8 +128,6 @@
   const std::string CheckName;
   const bool GetConfigPerFile;
   const bool IgnoreFailedSplit;
-
-  IdentifierNamingCheck::HungarianNotationOption HNOption;
 };
 
 } // namespace readability
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -230,22 +230,16 @@
 IdentifierNamingCheck::HungarianNotationOption &HNOption) {
 
   // Options
-  static constexpr std::pair Options[] = {
+  static constexpr std::pair General[] = {
   {"TreatStructAsClass", "false"}};
-  for (const auto &Opt : Options) {
-std::string Val = HNOption.General.lookup(Opt.first);
-if (Val.empty())
-  HNOption.General.insert({Opt.first, Opt.second.str()});
-  }
+  for (const auto &G : General)
+HNOption.General.try_emplace(G.first, G.second);
 
   // Derived types
   static constexpr std::pair DerivedTypes[] = {
   {"Array", "a"}, {"Pointer", "p"}, {"FunctionPointer", "fn"}};
-  for (const auto &Other : DerivedTypes) {
-std::string Val = HNOption.DerivedType.lookup(Other.first);
-if (Val.empty())
-  HNOption.DerivedType.insert({Other.first, Other.second.str()});
-  }
+  for (const auto &DT : DerivedTypes)
+HNOption.DerivedType.try_emplace(DT.first, DT.second);
 
   // C strings
   static constexpr std::pair CStrings[] = {
@@ -253,11 +247,8 @@
   {"char[]", "sz"},
   {"wchar_t*", "wsz"},
   {"wchar_t[]", "wsz"}};
-  for (const auto &CStr : CStrings) {
-std::string Val = HNOption.CString.lookup(CStr.first);
-if (Val.empty())
-  HNOption.CString.insert({CStr.first, CStr.second.str()});
-  }
+  for (const auto &CStr : CStrings)
+HNOption.CString.try_emplace(CStr.first, CStr.second);
 
   // clang-format off
   static constexpr std::pair PrimitiveTypes[] = {
@@ -305,11 +296,8 @@
 {"long","l"   },
 {"ptrdiff_t",   "p"   }};
   // clang-format on
-  for (const auto &Type : PrimitiveTypes) {
-std::string Val = HNOption.PrimitiveType.lookup(Type.first);
-if (Val.empty())
-  HNOption.PrimitiveType.insert({Type.first, Type.second.str()});
-  }
+  for (const auto &PT : PrimitiveTypes)
+HNOption.PrimitiveType.try_emplace(PT.first, PT.second);
 
   // clang-format off
   static constexpr std::pair UserDefinedTypes[] = {
@@ -343,11 +331,8 @@
   {"UINT64",  "u64" },
   {"PVOID",   "p"   } };
   // clang-format on
-  for (const auto &Type : UserDefinedTypes) {
-std::string Val = HNOption.UserDefinedType.lookup(Type.first);
-if (Val.empty())
-  HNOption.UserDefinedType.insert({Type.first, Type.second.str()});
-  }
+  for (const auto &UDT : UserDefinedTypes)
+HNOption.UserDefinedType.try_emplace(UDT.first, UDT.second);
 }
 
 static constexpr StringRef HNOpts[] = {"TreatStructAsClass"};
@@ -363,14 +348,14 @@
 Buffer.assign({Section, "General.", Opt});
 std::string Val = Options.get(Buffer, "");
 if (!Val

[PATCH] D92257: [clang-format] Add option to control the space at the front of a line comment

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D92257#2435902 , @MyDeveloperDay 
wrote:

> In D92257#2435899 , 
> @HazardyKnusperkeks wrote:
>
>> In D92257#2435701 , @MyDeveloperDay 
>> wrote:
>>
>>> Can I assume you need someone to land this for you?
>>
>> Yes I do. But I have a question, my last change is commited in your name, 
>> that means git blame would blame it on you, right?
>>
>> You can set me as author:
>> `Björn Schäpers `
>> My Github Account is also called `HazardyKnusperkeks`.
>
> The process is that you add (https://llvm.org/docs/Contributing.html)
>
> Patch By: HazardyKnusperkeks
>
> to the commit message if the user doesn't have commit access, if you want 
> your name against the blame then I recommend applying for commit access 
> yourself.

That is incorrect and does not represent the nowadays reality, i suggest that 
you look up the docs.

> let me know if you still want me to land this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92257

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

[PATCH] D92257: [clang-format] Add option to control the space at the front of a line comment

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

HazardyKnusperkeks added a comment.

In D92257#2435902 , @MyDeveloperDay 
wrote:

> In D92257#2435899 , 
> @HazardyKnusperkeks wrote:
>
>> In D92257#2435701 , @MyDeveloperDay 
>> wrote:
>>
>>> Can I assume you need someone to land this for you?
>>
>> Yes I do. But I have a question, my last change is commited in your name, 
>> that means git blame would blame it on you, right?
>>
>> You can set me as author:
>> `Björn Schäpers `
>> My Github Account is also called `HazardyKnusperkeks`.
>
> The process is that you add (https://llvm.org/docs/Contributing.html)
>
> Patch By: HazardyKnusperkeks
>
> to the commit message if the user doesn't have commit access, if you want 
> your name against the blame then I recommend applying for commit access 
> yourself.
>
> let me know if you still want me to land this

Updated. :)
Yes please land this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92257

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

[PATCH] D92257: [clang-format] Add option to control the space at the front of a line comment

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D92257#2435899 , 
@HazardyKnusperkeks wrote:

> In D92257#2435701 , @MyDeveloperDay 
> wrote:
>
>> Can I assume you need someone to land this for you?
>
> Yes I do. But I have a question, my last change is commited in your name, 
> that means git blame would blame it on you, right?
>
> You can set me as author:
> `Björn Schäpers `
> My Github Account is also called `HazardyKnusperkeks`.

The process is that you add (https://llvm.org/docs/Contributing.html)

Patch By: HazardyKnusperkeks

to the commit message if the user doesn't have commit access, if you want your 
name against the blame then I recommend applying for commit access yourself.

let me know if you still want me to land this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92257

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

[PATCH] D92596: [FPEnv] Correct constrained metadata in fp16-ops-strict.c

[Melanie Blower via Phabricator via cfe-commits]

mibintc added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2992
 // floating point environment in the loop.
+//XXX true?
 llvm::BasicBlock *startBB = Builder.GetInsertBlock();

did you mean to leave this here? (blame shows the fixme comment dates from 2012)



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3006
 
+  CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, OpInfo.FPFeatures);
   SourceLocation Loc = E->getExprLoc();

What's the rule to follow about when we need to FPOptsRAII? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92596

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

[PATCH] D92257: [clang-format] Add option to control the space at the front of a line comment

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

HazardyKnusperkeks added a comment.

In D92257#2435701 , @MyDeveloperDay 
wrote:

> Can I assume you need someone to land this for you?

Yes I do. But I have a question, my last change is commited in your name, that 
means git blame would blame it on you, right?

You can set me as author:
`Björn Schäpers `
My Github Account is also called `HazardyKnusperkeks`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92257

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

[PATCH] D92728: [NFC][MSan] Round up OffsetPtr in PoisonMembersgetFieldOffset(layoutStartOffset) for current calleds is expected topoint to the first trivial field or the one which follows non-trivial

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka created this revision.
vitalybuka added reviewers: morehouse, eugenis.
vitalybuka requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

...withoutassumptions about callers. This patch will avoid the need in 
suchassumptions.

Depends on D92727 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92728

Files:
  clang/lib/CodeGen/CGClass.cpp


Index: clang/lib/CodeGen/CGClass.cpp
===
--- clang/lib/CodeGen/CGClass.cpp
+++ clang/lib/CodeGen/CGClass.cpp
@@ -18,6 +18,7 @@
 #include "TargetInfo.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/CXXInheritance.h"
+#include "clang/AST/CharUnits.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/EvaluatedExprVisitor.h"
 #include "clang/AST/RecordLayout.h"
@@ -1729,37 +1730,35 @@
 /// \param layoutEndOffset index of the ASTRecordLayout field to
 /// end poisoning (exclusive)
 void PoisonMembers(CodeGenFunction &CGF, unsigned layoutStartOffset,
- unsigned layoutEndOffset) {
+   unsigned layoutEndOffset) {
   ASTContext &Context = CGF.getContext();
   const ASTRecordLayout &Layout =
   Context.getASTRecordLayout(Dtor->getParent());
 
-  llvm::ConstantInt *OffsetSizePtr = llvm::ConstantInt::get(
-  CGF.SizeTy,
-  Context.toCharUnitsFromBits(Layout.getFieldOffset(layoutStartOffset))
-  .getQuantity());
+  // It's a first trivia field so it should be at the begining of char,
+  // still round up start offset just in case.
+  CharUnits PoisonStart =
+  Context.toCharUnitsFromBits(Layout.getFieldOffset(layoutStartOffset) 
+
+  Context.getCharWidth() - 1);
+  llvm::ConstantInt *OffsetSizePtr =
+  llvm::ConstantInt::get(CGF.SizeTy, PoisonStart.getQuantity());
 
   llvm::Value *OffsetPtr = CGF.Builder.CreateGEP(
   CGF.Builder.CreateBitCast(CGF.LoadCXXThis(), CGF.Int8PtrTy),
   OffsetSizePtr);
 
-  CharUnits::QuantityType PoisonSize;
+  CharUnits PoisonEnd;
   if (layoutEndOffset >= Layout.getFieldCount()) {
-PoisonSize = Layout.getNonVirtualSize().getQuantity() -
- Context.toCharUnitsFromBits(
-Layout.getFieldOffset(layoutStartOffset))
- .getQuantity();
+PoisonEnd = Layout.getNonVirtualSize();
   } else {
-PoisonSize = Context.toCharUnitsFromBits(
-Layout.getFieldOffset(layoutEndOffset) -
-Layout.getFieldOffset(layoutStartOffset))
- .getQuantity();
+PoisonEnd =
+
Context.toCharUnitsFromBits(Layout.getFieldOffset(layoutEndOffset));
   }
-
-  if (PoisonSize == 0)
+  CharUnits PoisonSize = PoisonEnd - PoisonStart;
+  if (!PoisonSize.isPositive())
 return;
 
-  EmitSanitizerDtorCallback(CGF, OffsetPtr, PoisonSize);
+  EmitSanitizerDtorCallback(CGF, OffsetPtr, PoisonSize.getQuantity());
 }
   };
 


Index: clang/lib/CodeGen/CGClass.cpp
===
--- clang/lib/CodeGen/CGClass.cpp
+++ clang/lib/CodeGen/CGClass.cpp
@@ -18,6 +18,7 @@
 #include "TargetInfo.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/CXXInheritance.h"
+#include "clang/AST/CharUnits.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/EvaluatedExprVisitor.h"
 #include "clang/AST/RecordLayout.h"
@@ -1729,37 +1730,35 @@
 /// \param layoutEndOffset index of the ASTRecordLayout field to
 /// end poisoning (exclusive)
 void PoisonMembers(CodeGenFunction &CGF, unsigned layoutStartOffset,
- unsigned layoutEndOffset) {
+   unsigned layoutEndOffset) {
   ASTContext &Context = CGF.getContext();
   const ASTRecordLayout &Layout =
   Context.getASTRecordLayout(Dtor->getParent());
 
-  llvm::ConstantInt *OffsetSizePtr = llvm::ConstantInt::get(
-  CGF.SizeTy,
-  Context.toCharUnitsFromBits(Layout.getFieldOffset(layoutStartOffset))
-  .getQuantity());
+  // It's a first trivia field so it should be at the begining of char,
+  // still round up start offset just in case.
+  CharUnits PoisonStart =
+  Context.toCharUnitsFromBits(Layout.getFieldOffset(layoutStartOffset) +
+  Context.getCharWidth() - 1);
+  llvm::ConstantInt *OffsetSizePtr =
+  llvm::ConstantInt::get(CGF.SizeTy, PoisonStart.getQuantity());
 
   llvm::Value *OffsetPtr = CGF.Builder.CreateGEP(
   CGF.Builder.CreateBitCast(CGF.LoadCXXThis(), CGF.Int8PtrTy),
   OffsetSizePtr);
 
-  CharUnits::QuantityType PoisonSize;
+  CharUnits PoisonEnd;
   if (layoutEndOffset >= Layout.