[PATCH] D68720: Support -fstack-clash-protection for x86

[Kees Cook via Phabricator via cfe-commits]

kees added a comment.

Ah! Yes, I see it now. Thanks and sorry for the noise!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720

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

[PATCH] D86626: [OpenCL][Docs] 10.x release notes

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a subscriber: mantognini.
Anastasia added a comment.

@hans, would you be able to commit this to the release branch?

PS, as I am away from next week I am looping in @mantognini for any follow up.


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

https://reviews.llvm.org/D86626

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

[PATCH] D86765: [clang] Don't emit "no member" diagnostic if the lookup fails on an invalid record decl.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added projects: clang, libc++.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libc++.
hokein requested review of this revision.

The "no member" diagnostic is likely bogus.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86765

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/access-base-class.cpp
  
libcxx/test/libcxx/utilities/memory/util.smartptr/util.smartptr.shared/function_type_default_deleter.fail.cpp


Index: 
libcxx/test/libcxx/utilities/memory/util.smartptr/util.smartptr.shared/function_type_default_deleter.fail.cpp
===
--- 
libcxx/test/libcxx/utilities/memory/util.smartptr/util.smartptr.shared/function_type_default_deleter.fail.cpp
+++ 
libcxx/test/libcxx/utilities/memory/util.smartptr/util.smartptr.shared/function_type_default_deleter.fail.cpp
@@ -37,8 +37,6 @@
 SPtr<3> s3(nullptr, Deleter{}); // OK
   }
   // expected-error-re@memory:* 2 {{static_assert failed{{.*}} "default_delete 
cannot be instantiated for function types"}}
-  // FIXME: suppress this bogus diagnostic, see 
https://reviews.llvm.org/D86685.
-  // expected-error@memory:* 0+ {{no member named 'value' in}}
   {
 SPtr<4> s4(getFn<4>()); // expected-note {{requested here}}
 SPtr<5> s5(getFn<5>(), std::default_delete>{}); // expected-note 
{{requested here}}
Index: clang/test/SemaCXX/access-base-class.cpp
===
--- clang/test/SemaCXX/access-base-class.cpp
+++ clang/test/SemaCXX/access-base-class.cpp
@@ -89,3 +89,29 @@
   };
 }
 
+namespace T8 {
+template 
+struct flag {
+  static constexpr bool value = true;
+};
+
+template 
+struct trait : flag {};
+
+template ::value>
+struct a {};
+
+template 
+class b {
+  a x;
+  using U = a;
+};
+
+template 
+struct Impossible {
+  static_assert(false, ""); // expected-error {{static_assert failed}}
+};
+
+// verify "no member named 'value'" bogus diagnostic is not emitted.
+trait>>::value;
+} // namespace T8
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -2580,6 +2580,13 @@
  NameInfo, /*TemplateArgs=*/nullptr);
 
   if (R.empty()) {
+// Don't diagnose problems with invalid record decl, the no_member
+// diagnostic is likely bogus, e.g. if a base specificer is invalid, the
+// derived class is invalid, and has no base class attached, lookup of base
+// class members will fails.
+if (const auto *CD = dyn_cast(DC))
+  if (CD->isInvalidDecl())
+return ExprError();
 Diag(NameInfo.getLoc(), diag::err_no_member)
   << NameInfo.getName() << DC << SS.getRange();
 return ExprError();


Index: libcxx/test/libcxx/utilities/memory/util.smartptr/util.smartptr.shared/function_type_default_deleter.fail.cpp
===
--- libcxx/test/libcxx/utilities/memory/util.smartptr/util.smartptr.shared/function_type_default_deleter.fail.cpp
+++ libcxx/test/libcxx/utilities/memory/util.smartptr/util.smartptr.shared/function_type_default_deleter.fail.cpp
@@ -37,8 +37,6 @@
 SPtr<3> s3(nullptr, Deleter{}); // OK
   }
   // expected-error-re@memory:* 2 {{static_assert failed{{.*}} "default_delete cannot be instantiated for function types"}}
-  // FIXME: suppress this bogus diagnostic, see https://reviews.llvm.org/D86685.
-  // expected-error@memory:* 0+ {{no member named 'value' in}}
   {
 SPtr<4> s4(getFn<4>()); // expected-note {{requested here}}
 SPtr<5> s5(getFn<5>(), std::default_delete>{}); // expected-note {{requested here}}
Index: clang/test/SemaCXX/access-base-class.cpp
===
--- clang/test/SemaCXX/access-base-class.cpp
+++ clang/test/SemaCXX/access-base-class.cpp
@@ -89,3 +89,29 @@
   };
 }
 
+namespace T8 {
+template 
+struct flag {
+  static constexpr bool value = true;
+};
+
+template 
+struct trait : flag {};
+
+template ::value>
+struct a {};
+
+template 
+class b {
+  a x;
+  using U = a;
+};
+
+template 
+struct Impossible {
+  static_assert(false, ""); // expected-error {{static_assert failed}}
+};
+
+// verify "no member named 'value'" bogus diagnostic is not emitted.
+trait>>::value;
+} // namespace T8
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -2580,6 +2580,13 @@
  NameInfo, /*TemplateArgs=*/nullptr);
 
   if (R.empty()) {
+// Don't diagnose problems with invalid record decl, the no_member
+// diagnostic is likely bogus, e.g. if a base specificer is invalid, the
+// derived class is invalid, and has no base class attached, lookup of base
+// cl

[PATCH] D86688: [RecoveryExpr] Add 11.0.0 release note.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 288543.
hokein marked 4 inline comments as done.
hokein added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86688

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -48,6 +48,47 @@
 
 - ...
 
+Recovery AST
+
+
+clang's AST now improves support for representing broken C++ code. This 
improves
+the quality of subsequent diagnostics after an error is encountered. It also
+exposes more information to tools like clang-tidy and clangd that consume
+clang’s AST, allowing them to be more accurate on broken code.
+
+A RecoveryExpr is introduced in clang's AST, marking an expression containing
+semantic errors. This preserves the source range and subexpressions of the
+broken expression in the AST (rather than discarding the whole expression).
+
+For the following invalid code:
+
+  .. code-block:: c++
+
+ int NoArg(); // Line 1
+ int x = NoArg(42); // oops!
+
+clang-10 produces the minimal placeholder:
+
+  .. code-block:: c++
+
+ // VarDecl  col:5 x 'int'
+
+clang-11 produces a richer AST:
+
+  .. code-block:: c++
+
+ // VarDecl  col:5 x 'int' cinit
+ // `-RecoveryExpr  '' contains-errors 
lvalue
+ //`-UnresolvedLookupExpr  '' lvalue (ADL) 
= 'NoArg'
+ //`-IntegerLiteral  'int' 42
+
+Note that error-dependent types and values may now occur outside a template
+context. Tools may need to adjust assumptions about dependent code.
+
+This feature is on by default for C++ code, and can be explicitly controlled
+with `-Xclang -f[no-]recovery-ast`.
+
+
 Improvements to Clang's diagnostics
 ^^^
 


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -48,6 +48,47 @@
 
 - ...
 
+Recovery AST
+
+
+clang's AST now improves support for representing broken C++ code. This improves
+the quality of subsequent diagnostics after an error is encountered. It also
+exposes more information to tools like clang-tidy and clangd that consume
+clang’s AST, allowing them to be more accurate on broken code.
+
+A RecoveryExpr is introduced in clang's AST, marking an expression containing
+semantic errors. This preserves the source range and subexpressions of the
+broken expression in the AST (rather than discarding the whole expression).
+
+For the following invalid code:
+
+  .. code-block:: c++
+
+ int NoArg(); // Line 1
+ int x = NoArg(42); // oops!
+
+clang-10 produces the minimal placeholder:
+
+  .. code-block:: c++
+
+ // VarDecl  col:5 x 'int'
+
+clang-11 produces a richer AST:
+
+  .. code-block:: c++
+
+ // VarDecl  col:5 x 'int' cinit
+ // `-RecoveryExpr  '' contains-errors lvalue
+ //`-UnresolvedLookupExpr  '' lvalue (ADL) = 'NoArg'
+ //`-IntegerLiteral  'int' 42
+
+Note that error-dependent types and values may now occur outside a template
+context. Tools may need to adjust assumptions about dependent code.
+
+This feature is on by default for C++ code, and can be explicitly controlled
+with `-Xclang -f[no-]recovery-ast`.
+
+
 Improvements to Clang's diagnostics
 ^^^
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86688: [RecoveryExpr] Add 11.0.0 release note.

[Haojian Wu via Phabricator via cfe-commits]

hokein closed this revision.
hokein added a comment.

committed in 
https://github.com/llvm/llvm-project/commit/5d21aedfdbf0b85d65bad08b7b89913205de4b33.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86688

___
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

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Can you make sure you upload diffs with full context (-U9). Or using 
arcanist it will be done automatically.

Make sure the diff is up to date with trunk

Remove any changes that aren't related to this patch, they just make this look 
noisy.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D86671

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

[PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

[Pavel Labath via Phabricator via cfe-commits]

labath added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:1755-1759
+  QualType FromTy = ArrayFrom->getElementType();
+  QualType ToTy = ArrayTo->getElementType();
+
+  FromRecordDecl = FromTy->getAsRecordDecl();
+  ToRecordDecl = ToTy->getAsRecordDecl();

shafik wrote:
> martong wrote:
> > labath wrote:
> > > What about 2- or n-dimensional arrays?
> > @labath, this is a very good question! And made me realize that D71378 is 
> > fundamentally flawed (@shafik, please take no offense). Let me explain.
> > 
> > So, with D71378, we added the `if (ImportedOrErr) { ... }` block to import 
> > definitions specifically of fields' Record types. But we forget to handle 
> > arrays. Now we may forget to handle multidimensional arrays ... and we may 
> > forget to handle other language constructs. So, we would finally end up in 
> > re-implementing the logic of `ASTNodeImporter::VisitFieldDecl`.
> > 
> > So all this should have been handled properly by the preceding import call 
> > of the FieldDecl! Here
> > ```
> > 1735: ExpectedDecl ImportedOrErr = import(From);
> > ```
> > I have a suspicion that real reason why this import call fails in case of 
> > the public ASTImporter::ImportDefinition() is that we fail to drive through 
> > the import kind (`IDK_Everything`) during the import process.
> > Below we set IDK_Everything and start a complete import process.
> > ```
> >   8784   if (auto *ToRecord = dyn_cast(To)) {
> >   8785 if (!ToRecord->getDefinition()) {
> >   8786   return Importer.ImportDefinition(   // 
> > ASTNodeImporter::ImportDefinition !
> >   8787   cast(FromDC), ToRecord,
> >   8788   ASTNodeImporter::IDK_Everything);
> >   8789 }
> >   8790   }
> > ```
> > However, there may be many places where we fail to channel through that we 
> > want to do a complete import. E.g here
> > ```
> > 1957   
> > ImportDefinitionIfNeeded(Base1.getType()->getAsCXXRecordDecl()))
> > ```
> > we do another definition import and IDK_Everything is not set. So we may 
> > have a wrong kind of import since the "minimal" flag is set.
> > 
> > The thing is, it is really confusing and error-prone to have both the 
> > `ASTImporter::Minimal` flag and the `ImportDefinitionKind`. They seem to be 
> > in contradiction to each other some times.
> > I think we should get rid of the Minimal flag. And Kind should be either a 
> > full completion (IDK_Everythink) or just a minimal (IDK_Basic). So, I'd 
> > scrap the IDK_Default too. Alternatively we could have a Kind member in 
> > AST**//Node//**Importer.
> > I think we should go into this direction to avoid similar problems during 
> > CodeGen in the future. WDYT?
> No offense, you definitely understand the Importer better than I, so I value 
> your input here. I don't believe that should have other fields where we could 
> have a record that effects the layout but the concern is still valid and yes 
> I did miss multi-dimensional arrays.
> 
> Your theory on `IDK_Everything` not be pushed through everywhere, I did a 
> quick look and it seems pretty reasonable. 
> 
> I agree that the `ASTImporter::Minimal` flag and the `ImportDefinitionKind` 
> seem to inconsistent or perhaps a work-around. That seems like a bigger 
> change with a lot more impact beyond this fix but worth looking into longer 
> term. 
> 
> If pushing through `IDK_Everything` everywhere fixes the underlying issue I 
> am very happy to take that approach. If not we can discuss alternatives. 
I've been looking at this code, but I'm still not very familiar with it, so 
what I am asking may be obvious, but... What is the expected behavior for 
non-minimal imports for code like this?
```
struct A { ...};
struct B { A* a; }; // Note the pointer
```
Should importing B also import the definition of the A struct ? As I think that 
should not happen in the minimal import... If we get rid of the minimal flag, 
and rely solely on argument passing, we will need to be careful, as it 
shouldn't be passed _everywhere_ (it should stop at pointers for instance). But 
then it may not be possible to reproduce the current non-minimal import, as it 
(I think) expects that A will be fully imported too...

> I don't believe that should have other fields where we could have a record 
> that effects the layout
This isn't exactly layout related, but there is the question of covariant 
methods. If a method is covariant, then its return type must be complete. 
Currently we handle the completion of these in LLDB, but that solution is a bit 
hacky. It might be interesting if that could be handled by the ast importer as 
well.



Comment at: 
lldb/test/API/commands/expression/codegen-crash-import-def-arraytype-element/main.cpp:22-27
+  union {
+struct {
+  unsigned char *_s;
+} t;
+char *tt[1];
+  } U;

shafik wrote:
> labath wrote:
> > What's the significance of this union?
> Not sur

[PATCH] D86626: [OpenCL][Docs] 10.x release notes

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In D86626#2243876 , @Anastasia wrote:

> @hans, would you be able to commit this to the release branch?

Committed dae9fe408793def8a49f5e1d10d2a859627785e3 
. Thanks!


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

https://reviews.llvm.org/D86626

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

[PATCH] D86719: [SyntaxTree][NFC] Refactor function templates into functions taking base class

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 288559.
eduucaldas added a comment.

Move `getDeclaratorRange` helpers outside of `TreeBuilder`
Add coverage for qualified declarators and init-declarators


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86719

Files:
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -3123,6 +3123,35 @@
 )txt"}));
 }
 
+TEST_P(SyntaxTreeTest, OutOfLineMemberFunctionDefinition) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct S {
+  void f();
+};
+[[void S::f(){}]]
+)cpp",
+  {R"txt(
+SimpleDeclaration
+|-'void'
+|-SimpleDeclarator Declarator
+| |-NestedNameSpecifier
+| | |-IdentifierNameSpecifier ListElement
+| | | `-'S'
+| | `-'::' ListDelimiter
+| |-'f'
+| `-ParametersAndQualifiers
+|   |-'(' OpenParen
+|   `-')' CloseParen
+`-CompoundStatement
+  |-'{' OpenParen
+  `-'}' CloseParen
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest, ConversionMemberFunction) {
   if (!GetParam().isCXX()) {
 return;
@@ -3792,6 +3821,53 @@
 )txt"));
 }
 
+TEST_P(SyntaxTreeTest, InitDeclarator_Brace) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqual(
+  R"cpp(
+int a {};
+)cpp",
+  R"txt(
+TranslationUnit Detached
+`-SimpleDeclaration
+  |-'int'
+  |-SimpleDeclarator Declarator
+  | |-'a'
+  | `-UnknownExpression
+  |   `-UnknownExpression
+  | |-'{'
+  | `-'}'
+  `-';'
+)txt"));
+}
+
+TEST_P(SyntaxTreeTest, InitDeclarator_Paren) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct S {
+  S(int);
+};
+[[S s(1);]]
+)cpp",
+  {R"txt(
+SimpleDeclaration
+|-'S'
+|-SimpleDeclarator Declarator
+| `-UnknownExpression
+|   |-'s'
+|   |-'('
+|   |-IntegerLiteralExpression
+|   | `-'1' LiteralToken
+|   `-')'
+`-';'
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest, ArrayDeclarator_Simple) {
   EXPECT_TRUE(treeDumpEqual(
   R"cpp(
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -197,18 +197,57 @@
   llvm_unreachable("Unknown OverloadedOperatorKind enum");
 }
 
+/// Get the start of the qualified name. In the examples below it gives the
+/// location of the `^`:
+/// `int ^a;`
+/// `int ^a::S::f(){}`
+static SourceLocation getQualifiedNameStart(NamedDecl *D) {
+  assert((isa(D)) &&
+ "only DeclaratorDecl and TypedefNameDecl are supported.");
+
+  auto DN = D->getDeclName();
+  bool IsAnonymous = DN.isIdentifier() && !DN.getAsIdentifierInfo();
+  if (IsAnonymous)
+return SourceLocation();
+
+  if (const auto *DD = dyn_cast(D)) {
+if (DD->getQualifierLoc()) {
+  return DD->getQualifierLoc().getBeginLoc();
+}
+  }
+
+  return D->getLocation();
+}
+
+/// Gets the range of the initializer inside an init-declarator C++ [dcl.decl].
+/// `int a;` -> range of ``,
+/// `int *a = nullptr` -> range of `= nullptr`.
+/// `int a{}` -> range of `{}`.
+/// `int a()` -> range of `()`.
+static SourceRange getInitializerRange(Decl *D) {
+  if (auto *V = dyn_cast(D)) {
+auto *I = V->getInit();
+// Initializers in range-based-for are not part of the declarator
+if (I && !V->isCXXForRangeDecl())
+  return I->getSourceRange();
+  }
+
+  return SourceRange();
+}
+
 /// Gets the range of declarator as defined by the C++ grammar. E.g.
 /// `int a;` -> range of `a`,
 /// `int *a;` -> range of `*a`,
 /// `int a[10];` -> range of `a[10]`,
 /// `int a[1][2][3];` -> range of `a[1][2][3]`,
 /// `int *a = nullptr` -> range of `*a = nullptr`.
-/// FIMXE: \p Name must be a source range, e.g. for `operator+`.
+/// `int S::f(){}` -> range of `S::f()`.
+/// FIXME: \p Name must be a source range, e.g. for `operator+`.
 static SourceRange getDeclaratorRange(const SourceManager &SM, TypeLoc T,
   SourceLocation Name,
   SourceRange Initializer) {
   SourceLocation Start = GetStartLoc().Visit(T);
-  SourceLocation End = T.getSourceRange().getEnd();
+  SourceLocation End = T.getEndLoc();
   assert(End.isValid());
   if (Name.isValid()) {
 if (Start.isInvalid())
@@ -378,11 +417,9 @@
 
   /// Returns true if \p D is the last declarator in a chain and is thus
   /// reponsible for creating SimpleDeclaration for the whole chain.
-  template 
-  bool isResponsibleForCreatingDeclaration(const T *D) const {
-static_assert((std::is_base_of::value ||
-   std::is_base_of::value),
-   

[PATCH] D86719: [SyntaxTree][NFC] Refactor function templates into functions taking base class

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas added inline comments.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:225-226
+/// `int *a = nullptr` -> range of `= nullptr`.
+/// `int a{}` -> range of `{}`.
+/// `int a()` -> range of `()`.
+static SourceRange getInitializerRange(Decl *D) {

I followed the grammar to get these and also added tests for them.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:245
+/// `int S::f(){}` -> range of `S::f()`.
+/// FIXME: \p Name must be a source range, e.g. for `operator+`.
 static SourceRange getDeclaratorRange(const SourceManager &SM, TypeLoc T,

There was no problem with the declarator of `operator+` should I remove this?
Also I really tried to break it ^^.

I think though perhaps this is going to bite us in an obscure way, so I would 
just remove the `e.g. ...`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86719

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

[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given

[Hans Wennborg via Phabricator via cfe-commits]

hans added inline comments.



Comment at: clang/lib/Sema/SemaCast.cpp:895
+  if (!Self.getLangOpts().RTTIData) {
+bool isMSVC = Self.getDiagnostics().getDiagnosticOptions().getFormat() ==
+  DiagnosticOptions::MSVC;

zequanwu wrote:
> hans wrote:
> > I'm not sure isMSVC is the best name (it's not clear what aspect is MSVC 
> > exactly -- in this case it's the diagnostics format).
> > 
> > It's possible to target MSVC both with clang-cl and with regular clang.
> > 
> > For example, one could use
> > 
> >   clang-cl /c /tmp/a.cpp
> > 
> > or
> > 
> >   clang -c /tmp/a.cpp -target i686-pc-windows-msvc19.11.0 -fms-extensions
> > 
> > 
> > My understanding is that the purpose of "isMSVC" here is to try and detect 
> > if we're using clang-cl or clang so that the diagnostic can say "/GR-" or 
> > "-fno-rtti-data". So maybe it's better to call it "isClangCL" or something 
> > like that.
> > 
> > Also, I don't think we should check "isMSVC" in the if-statement below. We 
> > want the warning to fire both when using clang and clang-cl: as long as 
> > -fno-rtti-data or /GR- is used, the warning makes sense.
> > 
> > So I think the code could be more like:
> > 
> > ```
> > if (!Self.getLangOpts().RTTIData && !DestPointee->isVoidType()) {
> >   bool isClangCL = ...;
> >   Self.Diag(...) << isClangCL;
> > }
> > ```
> MSVC will warn even if the DestPointee is void type. What I thought is if 
> invoked by clang-cl warn regardless of DeskPointee type. If invoked by clang, 
> warn if it's not void type. 
> https://godbolt.org/z/475q5v. I noticed MSVC won't warn at typeid if /GR- is 
> given. Probably I should remove the warning in typeid.
If it's true the casting to void* doesn't need RTTI data (I think it is, but 
would be good to verify), then it doesn't make sense to warn. We don't have to 
follow MSVC's behavior when it doesn't make sense :)

Similar reasoning for typeid() - I assume it won't work with /GR- also with 
MSVC, so warning about it probably makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86369

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

[PATCH] D85105: [doxygen] Fix bad doxygen results for BugReporterVisitors.h

[Valeriy Savchenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9300ca541164: [doxygen] Fix bad doxygen results for 
BugReporterVisitors.h (authored by OikawaKirie, committed by vsavchenko).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85105

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h


Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
===
--- clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
+++ clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
@@ -53,7 +53,7 @@
   /// Note that this function does *not* get run on the very last node
   /// of the report, as the PathDiagnosticPiece associated with the
   /// last node should be unique.
-  /// Use {@code getEndPath} to customize the note associated with the report
+  /// Use \ref getEndPath to customize the note associated with the report
   /// end instead.
   ///
   /// The last parameter can be used to register a new visitor with the given


Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
===
--- clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
+++ clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
@@ -53,7 +53,7 @@
   /// Note that this function does *not* get run on the very last node
   /// of the report, as the PathDiagnosticPiece associated with the
   /// last node should be unique.
-  /// Use {@code getEndPath} to customize the note associated with the report
+  /// Use \ref getEndPath to customize the note associated with the report
   /// end instead.
   ///
   /// The last parameter can be used to register a new visitor with the given
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 9300ca5 - [doxygen] Fix bad doxygen results for BugReporterVisitors.h

[Valeriy Savchenko via cfe-commits]

Author: Ella Ma
Date: 2020-08-28T12:41:16+03:00
New Revision: 9300ca541164b80efb7d42dc45219b5d8e05de68

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

LOG: [doxygen] Fix bad doxygen results for BugReporterVisitors.h

`{@code x}` triggers a Doxygen bug. The bug may be matching the
close brace with the open brace of the namespace
declaration (`namespace clang {` or `namespace ento {`).

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h

Removed: 




diff  --git 
a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h 
b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
index 1aff2ca34a70..58a88f452ed9 100644
--- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
+++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
@@ -53,7 +53,7 @@ class BugReporterVisitor : public llvm::FoldingSetNode {
   /// Note that this function does *not* get run on the very last node
   /// of the report, as the PathDiagnosticPiece associated with the
   /// last node should be unique.
-  /// Use {@code getEndPath} to customize the note associated with the report
+  /// Use \ref getEndPath to customize the note associated with the report
   /// end instead.
   ///
   /// The last parameter can be used to register a new visitor with the given



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

[PATCH] D86716: [clang-format] Detect pointer qualifiers in cast expressions

[Alexander Richardson via Phabricator via cfe-commits]

arichardson updated this revision to Diff 288563.
arichardson added a comment.

drop unncessary const from test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86716

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8127,6 +8127,33 @@
AfterType);
 }
 
+TEST_F(FormatTest, UnderstandsPointerQualifiersInCast) {
+  // Check that qualifiers on pointers don't break parsing of casts.
+  verifyFormat("x = (foo *const)*v;");
+  verifyFormat("x = (foo *volatile)*v;");
+  verifyFormat("x = (foo *restrict)*v;");
+  verifyFormat("x = (foo *__attribute__((foo)))*v;");
+  verifyFormat("x = (foo *_Nonnull)*v;");
+  verifyFormat("x = (foo *_Nullable)*v;");
+  verifyFormat("x = (foo *_Null_unspecified)*v;");
+  verifyFormat("x = (foo *_Nonnull)*v;");
+
+  // Check that we handle multiple trailing qualifiers and skip them all to
+  // determine that the expression is a cast to a pointer type.
+  FormatStyle LongPointerRight = getLLVMStyleWithColumns(999);
+  FormatStyle LongPointerLeft = getLLVMStyleWithColumns(999);
+  LongPointerLeft.PointerAlignment = FormatStyle::PAS_Left;
+  StringRef AllQualifiers = "const volatile restrict __attribute__((foo)) "
+"_Nonnull _Null_unspecified _Nonnull";
+  verifyFormat(("x = (foo *" + AllQualifiers + ")*v;").str(), LongPointerRight);
+  verifyFormat(("x = (foo* " + AllQualifiers + ")*v;").str(), LongPointerLeft);
+
+  // Also check that address-of is not parsed as a binary bitwise-and:
+  verifyFormat("x = (foo *const)&v;");
+  verifyFormat(("x = (foo *" + AllQualifiers + ")&v;").str(), LongPointerRight);
+  verifyFormat(("x = (foo* " + AllQualifiers + ")&v;").str(), LongPointerLeft);
+}
+
 TEST_F(FormatTest, UnderstandsSquareAttributes) {
   verifyFormat("SomeType s [[unused]] (InitValue);");
   verifyFormat("SomeType s [[gnu::unused]] (InitValue);");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1827,10 +1827,30 @@
   return true;
 
 // Heuristically try to determine whether the parentheses contain a type.
-bool ParensAreType =
-!Tok.Previous ||
-Tok.Previous->isOneOf(TT_PointerOrReference, TT_TemplateCloser) ||
-Tok.Previous->isSimpleTypeSpecifier();
+auto IsQualifiedPointerOrReference = [](FormatToken *T) {
+  // This is used to handle cases such as x = (foo *const)&y;
+  assert(!T->isSimpleTypeSpecifier() && "Should have already been checked");
+  // Strip trailing qualifiers such as const or volatile when checking
+  // whether the parens could be a cast to a pointer/reference type.
+  while (T) {
+if (T->is(TT_AttributeParen)) {
+  // Handle `x = (foo *__attribute__((foo)))&v;`:
+  if (T->MatchingParen && T->MatchingParen->Previous &&
+  T->MatchingParen->Previous->is(tok::kw___attribute)) {
+T = T->MatchingParen->Previous->Previous;
+continue;
+  }
+} else if (T->canBePointerOrReferenceQualifier()) {
+  T = T->Previous;
+  continue;
+}
+break;
+  }
+  return T && T->is(TT_PointerOrReference);
+};
+bool ParensAreType = !Tok.Previous || Tok.Previous->is(TT_TemplateCloser) ||
+ Tok.Previous->isSimpleTypeSpecifier() ||
+ IsQualifiedPointerOrReference(Tok.Previous);
 bool ParensCouldEndDecl =
 Tok.Next->isOneOf(tok::equal, tok::semi, tok::l_brace, tok::greater);
 if (ParensAreType && !ParensCouldEndDecl)
@@ -1890,10 +1910,8 @@
 
 const FormatToken *NextToken = Tok.getNextNonComment();
 if (!NextToken ||
-NextToken->isOneOf(
-tok::arrow, tok::equal, tok::kw_const, tok::kw_restrict,
-tok::kw_volatile, tok::kw___attribute, tok::kw__Nonnull,
-tok::kw__Nullable, tok::kw__Null_unspecified, tok::kw_noexcept) ||
+NextToken->isOneOf(tok::arrow, tok::equal, tok::kw_noexcept) ||
+NextToken->canBePointerOrReferenceQualifier() ||
 (NextToken->is(tok::l_brace) && !NextToken->getNextNonComment()))
   return TT_PointerOrReference;
 
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -439,6 +439,12 @@
(!ColonRequired || (Next && Next->is(tok::colon)));
   }
 
+  bool canBePointerOrReferenceQualifier() const {
+return isOneOf(tok::kw_const, tok::kw_restrict, tok::kw_volatile,
+   tok:

[PATCH] D86721: [clang-format] Parse double-square attributes as pointer qualifiers

[Alexander Richardson via Phabricator via cfe-commits]

arichardson updated this revision to Diff 288564.
arichardson added a comment.

drop unnecesssary const


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86721

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8068,6 +8068,7 @@
   verifyIndependentOfContext("MACRO(A *_Null_unspecified a);");
   verifyIndependentOfContext("MACRO(A *__attribute__((foo)) a);");
   verifyIndependentOfContext("MACRO(A *__attribute((foo)) a);");
+  verifyIndependentOfContext("MACRO(A *[[clang::attr]] a);");
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
   // FIXME: Is there a way to make this work?
@@ -8137,14 +8138,16 @@
   verifyFormat("x = (foo *_Nullable)*v;");
   verifyFormat("x = (foo *_Null_unspecified)*v;");
   verifyFormat("x = (foo *_Nonnull)*v;");
+  verifyFormat("x = (foo *[[clang::attr]])*v;");
 
   // Check that we handle multiple trailing qualifiers and skip them all to
   // determine that the expression is a cast to a pointer type.
   FormatStyle LongPointerRight = getLLVMStyleWithColumns(999);
   FormatStyle LongPointerLeft = getLLVMStyleWithColumns(999);
   LongPointerLeft.PointerAlignment = FormatStyle::PAS_Left;
-  StringRef AllQualifiers = "const volatile restrict __attribute__((foo)) "
-"_Nonnull _Null_unspecified _Nonnull";
+  StringRef AllQualifiers =
+  "const volatile restrict __attribute__((foo)) _Nonnull _Null_unspecified 
"
+  "_Nonnull [[clang::attr]]";
   verifyFormat(("x = (foo *" + AllQualifiers + ")*v;").str(), 
LongPointerRight);
   verifyFormat(("x = (foo* " + AllQualifiers + ")*v;").str(), LongPointerLeft);
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1840,6 +1840,12 @@
 T = T->MatchingParen->Previous->Previous;
 continue;
   }
+} else if (T->is(TT_AttributeSquare)) {
+  // Handle `x = (foo *[[clang::foo]])&v;`:
+  if (T->MatchingParen && T->MatchingParen->Previous) {
+T = T->MatchingParen->Previous;
+continue;
+  }
 } else if (T->canBePointerOrReferenceQualifier()) {
   T = T->Previous;
   continue;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8068,6 +8068,7 @@
   verifyIndependentOfContext("MACRO(A *_Null_unspecified a);");
   verifyIndependentOfContext("MACRO(A *__attribute__((foo)) a);");
   verifyIndependentOfContext("MACRO(A *__attribute((foo)) a);");
+  verifyIndependentOfContext("MACRO(A *[[clang::attr]] a);");
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
   // FIXME: Is there a way to make this work?
@@ -8137,14 +8138,16 @@
   verifyFormat("x = (foo *_Nullable)*v;");
   verifyFormat("x = (foo *_Null_unspecified)*v;");
   verifyFormat("x = (foo *_Nonnull)*v;");
+  verifyFormat("x = (foo *[[clang::attr]])*v;");
 
   // Check that we handle multiple trailing qualifiers and skip them all to
   // determine that the expression is a cast to a pointer type.
   FormatStyle LongPointerRight = getLLVMStyleWithColumns(999);
   FormatStyle LongPointerLeft = getLLVMStyleWithColumns(999);
   LongPointerLeft.PointerAlignment = FormatStyle::PAS_Left;
-  StringRef AllQualifiers = "const volatile restrict __attribute__((foo)) "
-"_Nonnull _Null_unspecified _Nonnull";
+  StringRef AllQualifiers =
+  "const volatile restrict __attribute__((foo)) _Nonnull _Null_unspecified "
+  "_Nonnull [[clang::attr]]";
   verifyFormat(("x = (foo *" + AllQualifiers + ")*v;").str(), LongPointerRight);
   verifyFormat(("x = (foo* " + AllQualifiers + ")*v;").str(), LongPointerLeft);
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1840,6 +1840,12 @@
 T = T->MatchingParen->Previous->Previous;
 continue;
   }
+} else if (T->is(TT_AttributeSquare)) {
+  // Handle `x = (foo *[[clang::foo]])&v;`:
+  if (T->MatchingParen && T->MatchingParen->Previous) {
+T = T->MatchingParen->Previous;
+continue;
+  }
 } else if (T->canBePointerOrReferenceQualifier()) {
   T = T->Previous;
   continue;
___

[PATCH] D86711: [clang-format] Parse __attribute((foo)) as a pointer qualifier

[Alexander Richardson via Phabricator via cfe-commits]

arichardson marked 2 inline comments as done.
arichardson added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1895
tok::kw_restrict, tok::kw_volatile,
-   tok::kw_noexcept) ||
+   tok::kw___attribute, tok::kw_noexcept) ||
 (NextToken->is(tok::l_brace) && !NextToken->getNextNonComment()))

aaron.ballman wrote:
> arichardson wrote:
> > arichardson wrote:
> > > aaron.ballman wrote:
> > > > What about other attributes than GNU-style ones, like 
> > > > `[[clang::address_space(0)]]`?
> > > I guess those should also be handled. I can try to do that in a follow-up 
> > > change.
> > It appears that they are already handled for this case. I've handled them 
> > in cast expressions in D86721. 
> Ah, thank you for the update! Can you add a test case for that, as I don't 
> see one in the current tests?
I added the test to  D86721 since this change handles __attribute__ and that 
one deals with [[attr]]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86711

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

[PATCH] D86711: [clang-format] Parse __attribute((foo)) as a pointer qualifier

[MyDeveloperDay via Phabricator via cfe-commits]

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

LGTM thank you for these updates


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86711

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

[PATCH] D86716: [clang-format] Detect pointer qualifiers in cast expressions

[MyDeveloperDay via Phabricator via cfe-commits]

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

This LGTM thank you


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86716

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

[PATCH] D86065: [SVE] Make ElementCount members private

[Paul Walker via Phabricator via cfe-commits]

paulwalker-arm added a comment.

Can't say I agree since people are already writing the ugly code, because the 
result typically demands different handling or they're asserting the divide 
doesn't truncate in the first place.  That said I'm happy for there to be no 
assert as long as operator% is implemented so users can calculate the remainder 
in the expected way.


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

https://reviews.llvm.org/D86065

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

[PATCH] D86626: [OpenCL][Docs] 10.x release notes

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia closed this revision.
Anastasia added a comment.

https://reviews.llvm.org/rGdae9fe408793def8a49f5e1d10d2a859627785e3


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

https://reviews.llvm.org/D86626

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

[PATCH] D86708: [clang-format] Parse volatile as a pointer qualifier

[Alexander Richardson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1908da2658fc: [clang-format] Parse volatile as a pointer 
qualifier (authored by arichardson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86708

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8053,6 +8053,9 @@
   verifyIndependentOfContext("MACRO(auto *a);");
   verifyIndependentOfContext("MACRO(const A *a);");
   verifyIndependentOfContext("MACRO(A *const a);");
+  verifyIndependentOfContext("MACRO(A *volatile a);");
+  verifyIndependentOfContext("MACRO(A *__volatile a);");
+  verifyIndependentOfContext("MACRO(A *__volatile__ a);");
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
   // FIXME: Is there a way to make this work?
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1891,7 +1891,7 @@
 const FormatToken *NextToken = Tok.getNextNonComment();
 if (!NextToken ||
 NextToken->isOneOf(tok::arrow, tok::equal, tok::kw_const,
-   tok::kw_noexcept) ||
+   tok::kw_volatile, tok::kw_noexcept) ||
 (NextToken->is(tok::l_brace) && !NextToken->getNextNonComment()))
   return TT_PointerOrReference;
 


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8053,6 +8053,9 @@
   verifyIndependentOfContext("MACRO(auto *a);");
   verifyIndependentOfContext("MACRO(const A *a);");
   verifyIndependentOfContext("MACRO(A *const a);");
+  verifyIndependentOfContext("MACRO(A *volatile a);");
+  verifyIndependentOfContext("MACRO(A *__volatile a);");
+  verifyIndependentOfContext("MACRO(A *__volatile__ a);");
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
   // FIXME: Is there a way to make this work?
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1891,7 +1891,7 @@
 const FormatToken *NextToken = Tok.getNextNonComment();
 if (!NextToken ||
 NextToken->isOneOf(tok::arrow, tok::equal, tok::kw_const,
-   tok::kw_noexcept) ||
+   tok::kw_volatile, tok::kw_noexcept) ||
 (NextToken->is(tok::l_brace) && !NextToken->getNextNonComment()))
   return TT_PointerOrReference;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 96824ab - [clang-format] Detect pointer qualifiers in cast expressions

[Alex Richardson via cfe-commits]

Author: Alex Richardson
Date: 2020-08-28T11:31:47+01:00
New Revision: 96824abe7d80fc499032dab598968436132fcfb5

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

LOG: [clang-format] Detect pointer qualifiers in cast expressions

When guessing whether a closing paren is then end of a cast expression also
skip over pointer qualifiers while looking for TT_PointerOrReference.
This prevents some address-of and dereference operators from being parsed
as a binary operator.

Before:
x = (foo *const) * v;
x = (foo *const volatile restrict __attribute__((foo)) _Nonnull 
_Null_unspecified _Nonnull) & v;

After:
x = (foo *const)*v;
x = (foo *const volatile restrict __attribute__((foo)) _Nonnull 
_Null_unspecified _Nonnull)&v;

Reviewed By: MyDeveloperDay

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

Added: 


Modified: 
clang/lib/Format/FormatToken.h
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index ece1bf4b97f7..a54600a478a4 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -439,6 +439,12 @@ struct FormatToken {
(!ColonRequired || (Next && Next->is(tok::colon)));
   }
 
+  bool canBePointerOrReferenceQualifier() const {
+return isOneOf(tok::kw_const, tok::kw_restrict, tok::kw_volatile,
+   tok::kw___attribute, tok::kw__Nonnull, tok::kw__Nullable,
+   tok::kw__Null_unspecified);
+  }
+
   /// Determine whether the token is a simple-type-specifier.
   bool isSimpleTypeSpecifier() const;
 

diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 3e25ef6b9774..a9077500e041 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -1827,10 +1827,30 @@ class AnnotatingParser {
   return true;
 
 // Heuristically try to determine whether the parentheses contain a type.
-bool ParensAreType =
-!Tok.Previous ||
-Tok.Previous->isOneOf(TT_PointerOrReference, TT_TemplateCloser) ||
-Tok.Previous->isSimpleTypeSpecifier();
+auto IsQualifiedPointerOrReference = [](FormatToken *T) {
+  // This is used to handle cases such as x = (foo *const)&y;
+  assert(!T->isSimpleTypeSpecifier() && "Should have already been 
checked");
+  // Strip trailing qualifiers such as const or volatile when checking
+  // whether the parens could be a cast to a pointer/reference type.
+  while (T) {
+if (T->is(TT_AttributeParen)) {
+  // Handle `x = (foo *__attribute__((foo)))&v;`:
+  if (T->MatchingParen && T->MatchingParen->Previous &&
+  T->MatchingParen->Previous->is(tok::kw___attribute)) {
+T = T->MatchingParen->Previous->Previous;
+continue;
+  }
+} else if (T->canBePointerOrReferenceQualifier()) {
+  T = T->Previous;
+  continue;
+}
+break;
+  }
+  return T && T->is(TT_PointerOrReference);
+};
+bool ParensAreType = !Tok.Previous || Tok.Previous->is(TT_TemplateCloser) 
||
+ Tok.Previous->isSimpleTypeSpecifier() ||
+ IsQualifiedPointerOrReference(Tok.Previous);
 bool ParensCouldEndDecl =
 Tok.Next->isOneOf(tok::equal, tok::semi, tok::l_brace, tok::greater);
 if (ParensAreType && !ParensCouldEndDecl)
@@ -1890,10 +1910,8 @@ class AnnotatingParser {
 
 const FormatToken *NextToken = Tok.getNextNonComment();
 if (!NextToken ||
-NextToken->isOneOf(
-tok::arrow, tok::equal, tok::kw_const, tok::kw_restrict,
-tok::kw_volatile, tok::kw___attribute, tok::kw__Nonnull,
-tok::kw__Nullable, tok::kw__Null_unspecified, tok::kw_noexcept) ||
+NextToken->isOneOf(tok::arrow, tok::equal, tok::kw_noexcept) ||
+NextToken->canBePointerOrReferenceQualifier() ||
 (NextToken->is(tok::l_brace) && !NextToken->getNextNonComment()))
   return TT_PointerOrReference;
 

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 7e505c2401e9..f2978cdbed8d 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -8127,6 +8127,33 @@ TEST_F(FormatTest, UnderstandsAttributes) {
AfterType);
 }
 
+TEST_F(FormatTest, UnderstandsPointerQualifiersInCast) {
+  // Check that qualifiers on pointers don't break parsing of casts.
+  verifyFormat("x = (foo *const)*v;");
+  verifyFormat("x = (foo *volatile)*v;");
+  verifyFormat("x = (foo *restrict)*v;");
+  verifyFormat("x = (foo *__attribute__((foo)))*v;");
+  verifyFormat("x = (foo *_Nonnull)*v;");
+  verifyForm

[clang] d304360 - [clang-format] Parse nullability attributes as a pointer qualifier

[Alex Richardson via cfe-commits]

Author: Alex Richardson
Date: 2020-08-28T11:31:47+01:00
New Revision: d304360decefae3fa5c807a8cd0d7d4501a1cc4b

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

LOG: [clang-format] Parse nullability attributes as a pointer qualifier

Before:
void f() { MACRO(A * _Nonnull a); }
void f() { MACRO(A * _Nullable a); }
void f() { MACRO(A * _Null_unspecified a); }

After:
void f() { MACRO(A *_Nonnull a); }
void f() { MACRO(A *_Nullable a); }
void f() { MACRO(A *_Null_unspecified a); }

Reviewed By: JakeMerdichAMD

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

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index e5c0a7cb033c..3e25ef6b9774 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -1890,9 +1890,10 @@ class AnnotatingParser {
 
 const FormatToken *NextToken = Tok.getNextNonComment();
 if (!NextToken ||
-NextToken->isOneOf(tok::arrow, tok::equal, tok::kw_const,
-   tok::kw_restrict, tok::kw_volatile,
-   tok::kw___attribute, tok::kw_noexcept) ||
+NextToken->isOneOf(
+tok::arrow, tok::equal, tok::kw_const, tok::kw_restrict,
+tok::kw_volatile, tok::kw___attribute, tok::kw__Nonnull,
+tok::kw__Nullable, tok::kw__Null_unspecified, tok::kw_noexcept) ||
 (NextToken->is(tok::l_brace) && !NextToken->getNextNonComment()))
   return TT_PointerOrReference;
 

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 1234462a47ec..7e505c2401e9 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -8025,6 +8025,10 @@ TEST_F(FormatTest, UnderstandsUsesOfStarAndAmp) {
   verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("foo();");
   verifyFormat("foo();");
@@ -8059,6 +8063,9 @@ TEST_F(FormatTest, UnderstandsUsesOfStarAndAmp) {
   verifyIndependentOfContext("MACRO(A *volatile a);");
   verifyIndependentOfContext("MACRO(A *__volatile a);");
   verifyIndependentOfContext("MACRO(A *__volatile__ a);");
+  verifyIndependentOfContext("MACRO(A *_Nonnull a);");
+  verifyIndependentOfContext("MACRO(A *_Nullable a);");
+  verifyIndependentOfContext("MACRO(A *_Null_unspecified a);");
   verifyIndependentOfContext("MACRO(A *__attribute__((foo)) a);");
   verifyIndependentOfContext("MACRO(A *__attribute((foo)) a);");
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");



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

[clang] 4f10369 - [clang-format] Parse restrict as a pointer qualifier

[Alex Richardson via cfe-commits]

Author: Alex Richardson
Date: 2020-08-28T11:31:47+01:00
New Revision: 4f103695646bb9e16609a276072781f5a933570d

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

LOG: [clang-format] Parse restrict as a pointer qualifier

Before: void f() { MACRO(A * restrict a); }
After:  void f() { MACRO(A *restrict a); }

Also check that the __restrict and __restrict__ aliases are handled.

Reviewed By: JakeMerdichAMD

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

Added: 


Modified: 
clang/lib/Format/Format.cpp
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 8c1d7c90e02a..fe11cba9bfdf 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -2739,6 +2739,7 @@ LangOptions getFormattingLangOpts(const FormatStyle 
&Style) {
   LangOpts.ObjC = 1;
   LangOpts.MicrosoftExt = 1;// To get kw___try, kw___finally.
   LangOpts.DeclSpecKeyword = 1; // To get __declspec.
+  LangOpts.C99 = 1; // To get kw_restrict for non-underscore-prefixed restrict.
   return LangOpts;
 }
 

diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index bcd0bf913ecb..dfcd92dcd3e0 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -1891,7 +1891,8 @@ class AnnotatingParser {
 const FormatToken *NextToken = Tok.getNextNonComment();
 if (!NextToken ||
 NextToken->isOneOf(tok::arrow, tok::equal, tok::kw_const,
-   tok::kw_volatile, tok::kw_noexcept) ||
+   tok::kw_restrict, tok::kw_volatile,
+   tok::kw_noexcept) ||
 (NextToken->is(tok::l_brace) && !NextToken->getNextNonComment()))
   return TT_PointerOrReference;
 

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 7ea00ff33ca0..95fdbec4b97b 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -8053,6 +8053,9 @@ TEST_F(FormatTest, UnderstandsUsesOfStarAndAmp) {
   verifyIndependentOfContext("MACRO(auto *a);");
   verifyIndependentOfContext("MACRO(const A *a);");
   verifyIndependentOfContext("MACRO(A *const a);");
+  verifyIndependentOfContext("MACRO(A *restrict a);");
+  verifyIndependentOfContext("MACRO(A *__restrict__ a);");
+  verifyIndependentOfContext("MACRO(A *__restrict a);");
   verifyIndependentOfContext("MACRO(A *volatile a);");
   verifyIndependentOfContext("MACRO(A *__volatile a);");
   verifyIndependentOfContext("MACRO(A *__volatile__ a);");



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

[PATCH] D86710: [clang-format] Parse restrict as a pointer qualifier

[Alexander Richardson 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 rG4f103695646b: [clang-format] Parse restrict as a pointer 
qualifier (authored by arichardson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86710

Files:
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8053,6 +8053,9 @@
   verifyIndependentOfContext("MACRO(auto *a);");
   verifyIndependentOfContext("MACRO(const A *a);");
   verifyIndependentOfContext("MACRO(A *const a);");
+  verifyIndependentOfContext("MACRO(A *restrict a);");
+  verifyIndependentOfContext("MACRO(A *__restrict__ a);");
+  verifyIndependentOfContext("MACRO(A *__restrict a);");
   verifyIndependentOfContext("MACRO(A *volatile a);");
   verifyIndependentOfContext("MACRO(A *__volatile a);");
   verifyIndependentOfContext("MACRO(A *__volatile__ a);");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1891,7 +1891,8 @@
 const FormatToken *NextToken = Tok.getNextNonComment();
 if (!NextToken ||
 NextToken->isOneOf(tok::arrow, tok::equal, tok::kw_const,
-   tok::kw_volatile, tok::kw_noexcept) ||
+   tok::kw_restrict, tok::kw_volatile,
+   tok::kw_noexcept) ||
 (NextToken->is(tok::l_brace) && !NextToken->getNextNonComment()))
   return TT_PointerOrReference;
 
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2739,6 +2739,7 @@
   LangOpts.ObjC = 1;
   LangOpts.MicrosoftExt = 1;// To get kw___try, kw___finally.
   LangOpts.DeclSpecKeyword = 1; // To get __declspec.
+  LangOpts.C99 = 1; // To get kw_restrict for non-underscore-prefixed restrict.
   return LangOpts;
 }
 


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8053,6 +8053,9 @@
   verifyIndependentOfContext("MACRO(auto *a);");
   verifyIndependentOfContext("MACRO(const A *a);");
   verifyIndependentOfContext("MACRO(A *const a);");
+  verifyIndependentOfContext("MACRO(A *restrict a);");
+  verifyIndependentOfContext("MACRO(A *__restrict__ a);");
+  verifyIndependentOfContext("MACRO(A *__restrict a);");
   verifyIndependentOfContext("MACRO(A *volatile a);");
   verifyIndependentOfContext("MACRO(A *__volatile a);");
   verifyIndependentOfContext("MACRO(A *__volatile__ a);");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1891,7 +1891,8 @@
 const FormatToken *NextToken = Tok.getNextNonComment();
 if (!NextToken ||
 NextToken->isOneOf(tok::arrow, tok::equal, tok::kw_const,
-   tok::kw_volatile, tok::kw_noexcept) ||
+   tok::kw_restrict, tok::kw_volatile,
+   tok::kw_noexcept) ||
 (NextToken->is(tok::l_brace) && !NextToken->getNextNonComment()))
   return TT_PointerOrReference;
 
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2739,6 +2739,7 @@
   LangOpts.ObjC = 1;
   LangOpts.MicrosoftExt = 1;// To get kw___try, kw___finally.
   LangOpts.DeclSpecKeyword = 1; // To get __declspec.
+  LangOpts.C99 = 1; // To get kw_restrict for non-underscore-prefixed restrict.
   return LangOpts;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 1908da2 - [clang-format] Parse volatile as a pointer qualifier

[Alex Richardson via cfe-commits]

Author: Alex Richardson
Date: 2020-08-28T11:31:47+01:00
New Revision: 1908da2658fc26154e4103a50faeeca804c7c57d

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

LOG: [clang-format] Parse volatile as a pointer qualifier

Before: void f() { MACRO(A * volatile a); }
After:  void f() { MACRO(A *volatile a); }

Also check that the __volatile and __volatile__ aliases are handled.

Reviewed By: JakeMerdichAMD

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

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 11acb597aa40..bcd0bf913ecb 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -1891,7 +1891,7 @@ class AnnotatingParser {
 const FormatToken *NextToken = Tok.getNextNonComment();
 if (!NextToken ||
 NextToken->isOneOf(tok::arrow, tok::equal, tok::kw_const,
-   tok::kw_noexcept) ||
+   tok::kw_volatile, tok::kw_noexcept) ||
 (NextToken->is(tok::l_brace) && !NextToken->getNextNonComment()))
   return TT_PointerOrReference;
 

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index dcd9da77a390..7ea00ff33ca0 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -8053,6 +8053,9 @@ TEST_F(FormatTest, UnderstandsUsesOfStarAndAmp) {
   verifyIndependentOfContext("MACRO(auto *a);");
   verifyIndependentOfContext("MACRO(const A *a);");
   verifyIndependentOfContext("MACRO(A *const a);");
+  verifyIndependentOfContext("MACRO(A *volatile a);");
+  verifyIndependentOfContext("MACRO(A *__volatile a);");
+  verifyIndependentOfContext("MACRO(A *__volatile__ a);");
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
   // FIXME: Is there a way to make this work?



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

[PATCH] D86711: [clang-format] Parse __attribute((foo)) as a pointer qualifier

[Alexander Richardson via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
arichardson marked an inline comment as done.
Closed by commit rG37cdabdb82e3: [clang-format] Parse __attribute((foo)) as a 
pointer qualifier (authored by arichardson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86711

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8059,6 +8059,8 @@
   verifyIndependentOfContext("MACRO(A *volatile a);");
   verifyIndependentOfContext("MACRO(A *__volatile a);");
   verifyIndependentOfContext("MACRO(A *__volatile__ a);");
+  verifyIndependentOfContext("MACRO(A *__attribute__((foo)) a);");
+  verifyIndependentOfContext("MACRO(A *__attribute((foo)) a);");
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
   // FIXME: Is there a way to make this work?
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1892,7 +1892,7 @@
 if (!NextToken ||
 NextToken->isOneOf(tok::arrow, tok::equal, tok::kw_const,
tok::kw_restrict, tok::kw_volatile,
-   tok::kw_noexcept) ||
+   tok::kw___attribute, tok::kw_noexcept) ||
 (NextToken->is(tok::l_brace) && !NextToken->getNextNonComment()))
   return TT_PointerOrReference;
 


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8059,6 +8059,8 @@
   verifyIndependentOfContext("MACRO(A *volatile a);");
   verifyIndependentOfContext("MACRO(A *__volatile a);");
   verifyIndependentOfContext("MACRO(A *__volatile__ a);");
+  verifyIndependentOfContext("MACRO(A *__attribute__((foo)) a);");
+  verifyIndependentOfContext("MACRO(A *__attribute((foo)) a);");
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
   // FIXME: Is there a way to make this work?
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1892,7 +1892,7 @@
 if (!NextToken ||
 NextToken->isOneOf(tok::arrow, tok::equal, tok::kw_const,
tok::kw_restrict, tok::kw_volatile,
-   tok::kw_noexcept) ||
+   tok::kw___attribute, tok::kw_noexcept) ||
 (NextToken->is(tok::l_brace) && !NextToken->getNextNonComment()))
   return TT_PointerOrReference;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 37cdabd - [clang-format] Parse __attribute((foo)) as a pointer qualifier

[Alex Richardson via cfe-commits]

Author: Alex Richardson
Date: 2020-08-28T11:31:47+01:00
New Revision: 37cdabdb82e33e0d659c92a6cbb7049c31e0acbc

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

LOG: [clang-format] Parse __attribute((foo)) as a pointer qualifier

Before: void f() { MACRO(A * __attribute((foo)) a); }
After:  void f() { MACRO(A *__attribute((foo)) a); }

Also check that the __attribute__ alias is handled.

Reviewed By: MyDeveloperDay

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

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index dfcd92dcd3e0..e5c0a7cb033c 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -1892,7 +1892,7 @@ class AnnotatingParser {
 if (!NextToken ||
 NextToken->isOneOf(tok::arrow, tok::equal, tok::kw_const,
tok::kw_restrict, tok::kw_volatile,
-   tok::kw_noexcept) ||
+   tok::kw___attribute, tok::kw_noexcept) ||
 (NextToken->is(tok::l_brace) && !NextToken->getNextNonComment()))
   return TT_PointerOrReference;
 

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 95fdbec4b97b..1234462a47ec 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -8059,6 +8059,8 @@ TEST_F(FormatTest, UnderstandsUsesOfStarAndAmp) {
   verifyIndependentOfContext("MACRO(A *volatile a);");
   verifyIndependentOfContext("MACRO(A *__volatile a);");
   verifyIndependentOfContext("MACRO(A *__volatile__ a);");
+  verifyIndependentOfContext("MACRO(A *__attribute__((foo)) a);");
+  verifyIndependentOfContext("MACRO(A *__attribute((foo)) a);");
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
   // FIXME: Is there a way to make this work?



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

[PATCH] D86716: [clang-format] Detect pointer qualifiers in cast expressions

[Alexander Richardson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG96824abe7d80: [clang-format] Detect pointer qualifiers in 
cast expressions (authored by arichardson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86716

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8127,6 +8127,33 @@
AfterType);
 }
 
+TEST_F(FormatTest, UnderstandsPointerQualifiersInCast) {
+  // Check that qualifiers on pointers don't break parsing of casts.
+  verifyFormat("x = (foo *const)*v;");
+  verifyFormat("x = (foo *volatile)*v;");
+  verifyFormat("x = (foo *restrict)*v;");
+  verifyFormat("x = (foo *__attribute__((foo)))*v;");
+  verifyFormat("x = (foo *_Nonnull)*v;");
+  verifyFormat("x = (foo *_Nullable)*v;");
+  verifyFormat("x = (foo *_Null_unspecified)*v;");
+  verifyFormat("x = (foo *_Nonnull)*v;");
+
+  // Check that we handle multiple trailing qualifiers and skip them all to
+  // determine that the expression is a cast to a pointer type.
+  FormatStyle LongPointerRight = getLLVMStyleWithColumns(999);
+  FormatStyle LongPointerLeft = getLLVMStyleWithColumns(999);
+  LongPointerLeft.PointerAlignment = FormatStyle::PAS_Left;
+  StringRef AllQualifiers = "const volatile restrict __attribute__((foo)) "
+"_Nonnull _Null_unspecified _Nonnull";
+  verifyFormat(("x = (foo *" + AllQualifiers + ")*v;").str(), LongPointerRight);
+  verifyFormat(("x = (foo* " + AllQualifiers + ")*v;").str(), LongPointerLeft);
+
+  // Also check that address-of is not parsed as a binary bitwise-and:
+  verifyFormat("x = (foo *const)&v;");
+  verifyFormat(("x = (foo *" + AllQualifiers + ")&v;").str(), LongPointerRight);
+  verifyFormat(("x = (foo* " + AllQualifiers + ")&v;").str(), LongPointerLeft);
+}
+
 TEST_F(FormatTest, UnderstandsSquareAttributes) {
   verifyFormat("SomeType s [[unused]] (InitValue);");
   verifyFormat("SomeType s [[gnu::unused]] (InitValue);");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1827,10 +1827,30 @@
   return true;
 
 // Heuristically try to determine whether the parentheses contain a type.
-bool ParensAreType =
-!Tok.Previous ||
-Tok.Previous->isOneOf(TT_PointerOrReference, TT_TemplateCloser) ||
-Tok.Previous->isSimpleTypeSpecifier();
+auto IsQualifiedPointerOrReference = [](FormatToken *T) {
+  // This is used to handle cases such as x = (foo *const)&y;
+  assert(!T->isSimpleTypeSpecifier() && "Should have already been checked");
+  // Strip trailing qualifiers such as const or volatile when checking
+  // whether the parens could be a cast to a pointer/reference type.
+  while (T) {
+if (T->is(TT_AttributeParen)) {
+  // Handle `x = (foo *__attribute__((foo)))&v;`:
+  if (T->MatchingParen && T->MatchingParen->Previous &&
+  T->MatchingParen->Previous->is(tok::kw___attribute)) {
+T = T->MatchingParen->Previous->Previous;
+continue;
+  }
+} else if (T->canBePointerOrReferenceQualifier()) {
+  T = T->Previous;
+  continue;
+}
+break;
+  }
+  return T && T->is(TT_PointerOrReference);
+};
+bool ParensAreType = !Tok.Previous || Tok.Previous->is(TT_TemplateCloser) ||
+ Tok.Previous->isSimpleTypeSpecifier() ||
+ IsQualifiedPointerOrReference(Tok.Previous);
 bool ParensCouldEndDecl =
 Tok.Next->isOneOf(tok::equal, tok::semi, tok::l_brace, tok::greater);
 if (ParensAreType && !ParensCouldEndDecl)
@@ -1890,10 +1910,8 @@
 
 const FormatToken *NextToken = Tok.getNextNonComment();
 if (!NextToken ||
-NextToken->isOneOf(
-tok::arrow, tok::equal, tok::kw_const, tok::kw_restrict,
-tok::kw_volatile, tok::kw___attribute, tok::kw__Nonnull,
-tok::kw__Nullable, tok::kw__Null_unspecified, tok::kw_noexcept) ||
+NextToken->isOneOf(tok::arrow, tok::equal, tok::kw_noexcept) ||
+NextToken->canBePointerOrReferenceQualifier() ||
 (NextToken->is(tok::l_brace) && !NextToken->getNextNonComment()))
   return TT_PointerOrReference;
 
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -439,6 +439,12 @@
(!ColonRequired || (Next && Next->is(tok::colon)));
   }
 
+  bool canBePointerOrReferenceQualifier() const {
+return 

[PATCH] D86713: [clang-format] Parse nullability attributes as a pointer qualifier

[Alexander Richardson 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 rGd304360decef: [clang-format] Parse nullability attributes as 
a pointer qualifier (authored by arichardson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86713

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8025,6 +8025,10 @@
   verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("foo();");
   verifyFormat("foo();");
@@ -8059,6 +8063,9 @@
   verifyIndependentOfContext("MACRO(A *volatile a);");
   verifyIndependentOfContext("MACRO(A *__volatile a);");
   verifyIndependentOfContext("MACRO(A *__volatile__ a);");
+  verifyIndependentOfContext("MACRO(A *_Nonnull a);");
+  verifyIndependentOfContext("MACRO(A *_Nullable a);");
+  verifyIndependentOfContext("MACRO(A *_Null_unspecified a);");
   verifyIndependentOfContext("MACRO(A *__attribute__((foo)) a);");
   verifyIndependentOfContext("MACRO(A *__attribute((foo)) a);");
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1890,9 +1890,10 @@
 
 const FormatToken *NextToken = Tok.getNextNonComment();
 if (!NextToken ||
-NextToken->isOneOf(tok::arrow, tok::equal, tok::kw_const,
-   tok::kw_restrict, tok::kw_volatile,
-   tok::kw___attribute, tok::kw_noexcept) ||
+NextToken->isOneOf(
+tok::arrow, tok::equal, tok::kw_const, tok::kw_restrict,
+tok::kw_volatile, tok::kw___attribute, tok::kw__Nonnull,
+tok::kw__Nullable, tok::kw__Null_unspecified, tok::kw_noexcept) ||
 (NextToken->is(tok::l_brace) && !NextToken->getNextNonComment()))
   return TT_PointerOrReference;
 


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8025,6 +8025,10 @@
   verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("foo();");
   verifyFormat("foo();");
@@ -8059,6 +8063,9 @@
   verifyIndependentOfContext("MACRO(A *volatile a);");
   verifyIndependentOfContext("MACRO(A *__volatile a);");
   verifyIndependentOfContext("MACRO(A *__volatile__ a);");
+  verifyIndependentOfContext("MACRO(A *_Nonnull a);");
+  verifyIndependentOfContext("MACRO(A *_Nullable a);");
+  verifyIndependentOfContext("MACRO(A *_Null_unspecified a);");
   verifyIndependentOfContext("MACRO(A *__attribute__((foo)) a);");
   verifyIndependentOfContext("MACRO(A *__attribute((foo)) a);");
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1890,9 +1890,10 @@
 
 const FormatToken *NextToken = Tok.getNextNonComment();
 if (!NextToken ||
-NextToken->isOneOf(tok::arrow, tok::equal, tok::kw_const,
-   tok::kw_restrict, tok::kw_volatile,
-   tok::kw___attribute, tok::kw_noexcept) ||
+NextToken->isOneOf(
+tok::arrow, tok::equal, tok::kw_const, tok::kw_restrict,
+tok::kw_volatile, tok::kw___attribute, tok::kw__Nonnull,
+tok::kw__Nullable, tok::kw__Null_unspecified, tok::kw_noexcept) ||
 (NextToken->is(tok::l_brace) && !NextToken->getNextNonComment()))
   return TT_PointerOrReference;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86065: [SVE] Make ElementCount members private

[Paul Walker via Phabricator via cfe-commits]

paulwalker-arm added a comment.

I'm retracting my operator% request. After thinking about it and speaking with 
Dave I just cannot see how allowing a total divide is safe for scalable 
vectors.  If you are relying on a truncating divide then special handling is 
require anyway, which is likely to be different between fixed-length and 
scalable vectors.


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

https://reviews.llvm.org/D86065

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

[PATCH] D86719: [SyntaxTree][NFC] Refactor function templates into functions taking base class

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 accepted this revision.
gribozavr2 added inline comments.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:202
+/// location of the `^`:
+/// `int ^a;`
+/// `int ^a::S::f(){}`





Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:245
+/// `int S::f(){}` -> range of `S::f()`.
+/// FIXME: \p Name must be a source range, e.g. for `operator+`.
 static SourceRange getDeclaratorRange(const SourceManager &SM, TypeLoc T,

eduucaldas wrote:
> There was no problem with the declarator of `operator+` should I remove this?
> Also I really tried to break it ^^.
> 
> I think though perhaps this is going to bite us in an obscure way, so I would 
> just remove the `e.g. ...`.
Feel free to remove the fixme.

What do you mean by "remove the e.g. ..."? Removing the examples from the 
comment? Why?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86719

___
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 added a comment.

In D86671#2243980 , @njames93 wrote:

> Can you make sure you upload diffs with full context (-U9). Or using 
> arcanist it will be done automatically.
>
> Make sure the diff is up to date with trunk
>
> Remove any changes that aren't related to this patch, they just make this 
> look noisy.

Hi @njames93,

1. It is my first time to use Phabricator and Arcanist, I will check how to 
make full context (-U9).
2. Do you mean that always sync with the latest master?
3. OK.




Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:317
+  if (Style.Case == IdentifierNamingCheck::CaseType::CT_HungarianNotation) {
+const auto TypePrefix = getHungarianNotationTypePrefix(Type.str(), Decl);
+if (TypePrefix.length() > 0) {

Eugene.Zelenko wrote:
> Please don't use auto when type is not specified explicitly in same statement 
> or iterator.
Got it, I fixed it in the next commit. Thank you.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:430
+const NamedDecl *pNamedDecl = dyn_cast(pDecl);
+const auto TypePrefix =
+getHungarianNotationTypePrefix(Type.str(), pNamedDecl);

Eugene.Zelenko wrote:
> Please don't use auto when type is not specified explicitly in same statement 
> or iterator.
Got it, I fixed it in the next commit. Thank you.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:70
 
+* Added: Add IdentifierNamingCheck::CaseType, CT_HungarianNotation, supporting 
naming check with Hungarian notation.
+

Eugene.Zelenko wrote:
> Please move to `Changes in existing checks` section (in alphabetical order).
> 
> I think statement should describe user-facing changes.
OK~ Fix it in next commit.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:76
 
-- Improved :doc:`readability-identifier-naming
+- Improved: doc:`readability-identifier-naming
   ` check.

Eugene.Zelenko wrote:
> Unrelated change.
OK~ Fix it on next commit.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D86671

___
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 added a comment.

In D86671#2243788 , @Eugene.Zelenko 
wrote:

> It'll be good idea to add test case.

Hi @Eugene.Zelenko,
I have created a `readability-identifier-naming-hungarain-notion.cpp`  file and 
several test cases for regression testing. Is it regression testing?

Find it here (https://reviews.llvm.org/differential/changeset/?ref=2137882)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D86671

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

[PATCH] D86775: [clang-format] Parse __ptr32/__ptr64 as a pointer qualifier

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: MyDeveloperDay, JakeMerdichAMD, sammccall, 
curdeius.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
arichardson requested review of this revision.

Before:
x = (foo *__ptr32) * v;
MACRO(A * __ptr32 a);
x = (foo *__ptr64) * v;
MACRO(A * __ptr64 a);

After:
x = (foo *__ptr32)*v;
MACRO(A *__ptr32 a);
x = (foo *__ptr64)*v;
MACRO(A *__ptr64 a);

Depends on D86721  (to apply cleanly)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86775

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8028,6 +8028,8 @@
   verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("foo();");
@@ -8069,6 +8071,8 @@
   verifyIndependentOfContext("MACRO(A *__attribute__((foo)) a);");
   verifyIndependentOfContext("MACRO(A *__attribute((foo)) a);");
   verifyIndependentOfContext("MACRO(A *[[clang::attr]] a);");
+  verifyIndependentOfContext("MACRO(A *__ptr32 a);");
+  verifyIndependentOfContext("MACRO(A *__ptr64 a);");
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
   // FIXME: Is there a way to make this work?
@@ -8139,6 +8143,8 @@
   verifyFormat("x = (foo *_Null_unspecified)*v;");
   verifyFormat("x = (foo *_Nonnull)*v;");
   verifyFormat("x = (foo *[[clang::attr]])*v;");
+  verifyFormat("x = (foo *__ptr32)*v;");
+  verifyFormat("x = (foo *__ptr64)*v;");
 
   // Check that we handle multiple trailing qualifiers and skip them all to
   // determine that the expression is a cast to a pointer type.
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -442,7 +442,7 @@
   bool canBePointerOrReferenceQualifier() const {
 return isOneOf(tok::kw_const, tok::kw_restrict, tok::kw_volatile,
tok::kw___attribute, tok::kw__Nonnull, tok::kw__Nullable,
-   tok::kw__Null_unspecified);
+   tok::kw__Null_unspecified, tok::kw___ptr32, 
tok::kw___ptr64);
   }
 
   /// Determine whether the token is a simple-type-specifier.


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8028,6 +8028,8 @@
   verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("foo();");
@@ -8069,6 +8071,8 @@
   verifyIndependentOfContext("MACRO(A *__attribute__((foo)) a);");
   verifyIndependentOfContext("MACRO(A *__attribute((foo)) a);");
   verifyIndependentOfContext("MACRO(A *[[clang::attr]] a);");
+  verifyIndependentOfContext("MACRO(A *__ptr32 a);");
+  verifyIndependentOfContext("MACRO(A *__ptr64 a);");
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
   // FIXME: Is there a way to make this work?
@@ -8139,6 +8143,8 @@
   verifyFormat("x = (foo *_Null_unspecified)*v;");
   verifyFormat("x = (foo *_Nonnull)*v;");
   verifyFormat("x = (foo *[[clang::attr]])*v;");
+  verifyFormat("x = (foo *__ptr32)*v;");
+  verifyFormat("x = (foo *__ptr64)*v;");
 
   // Check that we handle multiple trailing qualifiers and skip them all to
   // determine that the expression is a cast to a pointer type.
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -442,7 +442,7 @@
   bool canBePointerOrReferenceQualifier() const {
 return isOneOf(tok::kw_const, tok::kw_restrict, tok::kw_volatile,
tok::kw___attribute, tok::kw__Nonnull, tok::kw__Nullable,
-   tok::kw__Null_unspecified);
+   tok::kw__Null_unspecified, tok::kw___ptr32, tok::kw___ptr64);
   }
 
   /// Determine whether the token is a simple-type-specifier.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86778: Extract infrastructure to ignore intermediate expressions into `clang/AST/IgnoreExpr.h`

[Eduardo Caldas via Phabricator via cfe-commits]

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

This allows users to use `IgnoreExprNodes` outside of `clang/AST/Expr.h`


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86778

Files:
  clang/include/clang/AST/IgnoreExpr.h
  clang/lib/AST/Expr.cpp


Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/DependenceFlags.h"
 #include "clang/AST/EvaluatedExprVisitor.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/IgnoreExpr.h"
 #include "clang/AST/Mangle.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/StmtVisitor.h"
@@ -2914,25 +2915,6 @@
   return E;
 }
 
-static Expr *IgnoreExprNodesImpl(Expr *E) { return E; }
-template 
-static Expr *IgnoreExprNodesImpl(Expr *E, FnTy &&Fn, FnTys &&... Fns) {
-  return IgnoreExprNodesImpl(Fn(E), std::forward(Fns)...);
-}
-
-/// Given an expression E and functions Fn_1,...,Fn_n : Expr * -> Expr *,
-/// Recursively apply each of the functions to E until reaching a fixed point.
-/// Note that a null E is valid; in this case nothing is done.
-template 
-static Expr *IgnoreExprNodes(Expr *E, FnTys &&... Fns) {
-  Expr *LastE = nullptr;
-  while (E != LastE) {
-LastE = E;
-E = IgnoreExprNodesImpl(E, std::forward(Fns)...);
-  }
-  return E;
-}
-
 Expr *Expr::IgnoreImpCasts() {
   return IgnoreExprNodes(this, IgnoreImpCastsSingleStep);
 }
Index: clang/include/clang/AST/IgnoreExpr.h
===
--- /dev/null
+++ clang/include/clang/AST/IgnoreExpr.h
@@ -0,0 +1,36 @@
+//===--- IgnoreExpr.h - Ignore intermediate Expressions -===//
+//
+// 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 defines common functions to ignore intermediate expression nodes
+//
+//===--===//
+#include "clang/AST/Expr.h"
+
+namespace clang {
+namespace {
+/// Given an expression E and functions Fn_1,...,Fn_n : Expr * -> Expr *,
+/// Return Fn_n(...(Fn_1(E)))
+Expr *IgnoreExprNodesImpl(Expr *E) { return E; }
+template 
+Expr *IgnoreExprNodesImpl(Expr *E, FnTy &&Fn, FnTys &&...Fns) {
+  return IgnoreExprNodesImpl(Fn(E), std::forward(Fns)...);
+}
+} // namespace
+
+/// Given an expression E and functions Fn_1,...,Fn_n : Expr * -> Expr *,
+/// Recursively apply each of the functions to E until reaching a fixed point.
+/// Note that a null E is valid; in this case nothing is done.
+template  Expr *IgnoreExprNodes(Expr *E, FnTys &&...Fns) {
+  Expr *LastE = nullptr;
+  while (E != LastE) {
+LastE = E;
+E = IgnoreExprNodesImpl(E, std::forward(Fns)...);
+  }
+  return E;
+}
+} // namespace clang


Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/DependenceFlags.h"
 #include "clang/AST/EvaluatedExprVisitor.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/IgnoreExpr.h"
 #include "clang/AST/Mangle.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/StmtVisitor.h"
@@ -2914,25 +2915,6 @@
   return E;
 }
 
-static Expr *IgnoreExprNodesImpl(Expr *E) { return E; }
-template 
-static Expr *IgnoreExprNodesImpl(Expr *E, FnTy &&Fn, FnTys &&... Fns) {
-  return IgnoreExprNodesImpl(Fn(E), std::forward(Fns)...);
-}
-
-/// Given an expression E and functions Fn_1,...,Fn_n : Expr * -> Expr *,
-/// Recursively apply each of the functions to E until reaching a fixed point.
-/// Note that a null E is valid; in this case nothing is done.
-template 
-static Expr *IgnoreExprNodes(Expr *E, FnTys &&... Fns) {
-  Expr *LastE = nullptr;
-  while (E != LastE) {
-LastE = E;
-E = IgnoreExprNodesImpl(E, std::forward(Fns)...);
-  }
-  return E;
-}
-
 Expr *Expr::IgnoreImpCasts() {
   return IgnoreExprNodes(this, IgnoreImpCastsSingleStep);
 }
Index: clang/include/clang/AST/IgnoreExpr.h
===
--- /dev/null
+++ clang/include/clang/AST/IgnoreExpr.h
@@ -0,0 +1,36 @@
+//===--- IgnoreExpr.h - Ignore intermediate Expressions -===//
+//
+// 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 defines common functions to ignore intermediate expression nodes
+//
+//===-

[PATCH] D86778: Extract infrastructure to ignore intermediate expressions into `clang/AST/IgnoreExpr.h`

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

> This allows users to use IgnoreExprNodes outside of clang/AST/Expr.h

Did you mean "outside of Expr.cpp"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86778

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

[PATCH] D86719: [SyntaxTree][NFC] Refactor function templates into functions taking base class

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 288584.
eduucaldas marked an inline comment as done.
eduucaldas added a comment.

Rebase and answer comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86719

Files:
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -3123,6 +3123,35 @@
 )txt"}));
 }
 
+TEST_P(SyntaxTreeTest, OutOfLineMemberFunctionDefinition) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct S {
+  void f();
+};
+[[void S::f(){}]]
+)cpp",
+  {R"txt(
+SimpleDeclaration
+|-'void'
+|-SimpleDeclarator Declarator
+| |-NestedNameSpecifier
+| | |-IdentifierNameSpecifier ListElement
+| | | `-'S'
+| | `-'::' ListDelimiter
+| |-'f'
+| `-ParametersAndQualifiers
+|   |-'(' OpenParen
+|   `-')' CloseParen
+`-CompoundStatement
+  |-'{' OpenParen
+  `-'}' CloseParen
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest, ConversionMemberFunction) {
   if (!GetParam().isCXX()) {
 return;
@@ -3792,6 +3821,53 @@
 )txt"));
 }
 
+TEST_P(SyntaxTreeTest, InitDeclarator_Brace) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqual(
+  R"cpp(
+int a {};
+)cpp",
+  R"txt(
+TranslationUnit Detached
+`-SimpleDeclaration
+  |-'int'
+  |-SimpleDeclarator Declarator
+  | |-'a'
+  | `-UnknownExpression
+  |   `-UnknownExpression
+  | |-'{'
+  | `-'}'
+  `-';'
+)txt"));
+}
+
+TEST_P(SyntaxTreeTest, InitDeclarator_Paren) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct S {
+  S(int);
+};
+[[S s(1);]]
+)cpp",
+  {R"txt(
+SimpleDeclaration
+|-'S'
+|-SimpleDeclarator Declarator
+| `-UnknownExpression
+|   |-'s'
+|   |-'('
+|   |-IntegerLiteralExpression
+|   | `-'1' LiteralToken
+|   `-')'
+`-';'
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest, ArrayDeclarator_Simple) {
   EXPECT_TRUE(treeDumpEqual(
   R"cpp(
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -197,18 +197,58 @@
   llvm_unreachable("Unknown OverloadedOperatorKind enum");
 }
 
+/// Get the start of the qualified name. In the examples below it gives the
+/// location of the `^`:
+/// `int ^a;`
+/// `int *^a;`
+/// `int ^a::S::f(){}`
+static SourceLocation getQualifiedNameStart(NamedDecl *D) {
+  assert((isa(D)) &&
+ "only DeclaratorDecl and TypedefNameDecl are supported.");
+
+  auto DN = D->getDeclName();
+  bool IsAnonymous = DN.isIdentifier() && !DN.getAsIdentifierInfo();
+  if (IsAnonymous)
+return SourceLocation();
+
+  if (const auto *DD = dyn_cast(D)) {
+if (DD->getQualifierLoc()) {
+  return DD->getQualifierLoc().getBeginLoc();
+}
+  }
+
+  return D->getLocation();
+}
+
+/// Gets the range of the initializer inside an init-declarator C++ [dcl.decl].
+/// `int a;` -> range of ``,
+/// `int *a = nullptr` -> range of `= nullptr`.
+/// `int a{}` -> range of `{}`.
+/// `int a()` -> range of `()`.
+static SourceRange getInitializerRange(Decl *D) {
+  if (auto *V = dyn_cast(D)) {
+auto *I = V->getInit();
+// Initializers in range-based-for are not part of the declarator
+if (I && !V->isCXXForRangeDecl())
+  return I->getSourceRange();
+  }
+
+  return SourceRange();
+}
+
 /// Gets the range of declarator as defined by the C++ grammar. E.g.
 /// `int a;` -> range of `a`,
 /// `int *a;` -> range of `*a`,
 /// `int a[10];` -> range of `a[10]`,
 /// `int a[1][2][3];` -> range of `a[1][2][3]`,
 /// `int *a = nullptr` -> range of `*a = nullptr`.
-/// FIMXE: \p Name must be a source range, e.g. for `operator+`.
+/// `int S::f(){}` -> range of `S::f()`.
+/// FIXME: \p Name must be a source range.
 static SourceRange getDeclaratorRange(const SourceManager &SM, TypeLoc T,
   SourceLocation Name,
   SourceRange Initializer) {
   SourceLocation Start = GetStartLoc().Visit(T);
-  SourceLocation End = T.getSourceRange().getEnd();
+  SourceLocation End = T.getEndLoc();
   assert(End.isValid());
   if (Name.isValid()) {
 if (Start.isInvalid())
@@ -378,11 +418,9 @@
 
   /// Returns true if \p D is the last declarator in a chain and is thus
   /// reponsible for creating SimpleDeclaration for the whole chain.
-  template 
-  bool isResponsibleForCreatingDeclaration(const T *D) const {
-static_assert((std::is_base_of::value ||
-   std::is_base_of::value),
-  "only DeclaratorDecl and TypedefNameDecl are s

[PATCH] D86719: [SyntaxTree][NFC] Refactor function templates into functions taking base class

[Eduardo Caldas via Phabricator via cfe-commits]

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



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:245
+/// `int S::f(){}` -> range of `S::f()`.
+/// FIXME: \p Name must be a source range, e.g. for `operator+`.
 static SourceRange getDeclaratorRange(const SourceManager &SM, TypeLoc T,

gribozavr2 wrote:
> eduucaldas wrote:
> > There was no problem with the declarator of `operator+` should I remove 
> > this?
> > Also I really tried to break it ^^.
> > 
> > I think though perhaps this is going to bite us in an obscure way, so I 
> > would just remove the `e.g. ...`.
> Feel free to remove the fixme.
> 
> What do you mean by "remove the e.g. ..."? Removing the examples from the 
> comment? Why?
I meant 
```
/// FIXME: \p Name must be a source range, e.g. for `operator+`.
```
->
```
/// FIXME: \p Name must be a source range.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86719

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

[PATCH] D86065: [SVE] Make ElementCount members private

[Paul Walker via Phabricator via cfe-commits]

paulwalker-arm added a comment.

To be more clear, I'm happy to defer the divide conversation for if/when we run 
into issues so my previous acceptance still stands.  It'll be good to get the 
intent of the patch in (i.e. stoping access to internal class members) asap, 
plus any follow up work will be a smaller more manageable patch.  It's worth 
talking this through during the next sync call to see it we can get some 
consensus regarding what maths is and isn't allowed.


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

https://reviews.llvm.org/D86065

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

[PATCH] D86719: [SyntaxTree][NFC] Refactor function templates into functions taking base class

[Eduardo Caldas via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
eduucaldas marked an inline comment as done.
Closed by commit rG38bc0060e60f: [SyntaxTree][NFC] Refactor function templates 
into functions taking base class (authored by eduucaldas).

Changed prior to commit:
  https://reviews.llvm.org/D86719?vs=288584&id=288586#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86719

Files:
  clang/lib/Tooling/Syntax/BuildTree.cpp

Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -197,18 +197,57 @@
   llvm_unreachable("Unknown OverloadedOperatorKind enum");
 }
 
+/// Get the start of the qualified name. In the examples below it gives the
+/// location of the `^`:
+/// `int ^a;`
+/// `int ^a::S::f(){}`
+static SourceLocation getQualifiedNameStart(NamedDecl *D) {
+  assert((isa(D)) &&
+ "only DeclaratorDecl and TypedefNameDecl are supported.");
+
+  auto DN = D->getDeclName();
+  bool IsAnonymous = DN.isIdentifier() && !DN.getAsIdentifierInfo();
+  if (IsAnonymous)
+return SourceLocation();
+
+  if (const auto *DD = dyn_cast(D)) {
+if (DD->getQualifierLoc()) {
+  return DD->getQualifierLoc().getBeginLoc();
+}
+  }
+
+  return D->getLocation();
+}
+
+/// Gets the range of the initializer inside an init-declarator C++ [dcl.decl].
+/// `int a;` -> range of ``,
+/// `int *a = nullptr` -> range of `= nullptr`.
+/// `int a{}` -> range of `{}`.
+/// `int a()` -> range of `()`.
+static SourceRange getInitializerRange(Decl *D) {
+  if (auto *V = dyn_cast(D)) {
+auto *I = V->getInit();
+// Initializers in range-based-for are not part of the declarator
+if (I && !V->isCXXForRangeDecl())
+  return I->getSourceRange();
+  }
+
+  return SourceRange();
+}
+
 /// Gets the range of declarator as defined by the C++ grammar. E.g.
 /// `int a;` -> range of `a`,
 /// `int *a;` -> range of `*a`,
 /// `int a[10];` -> range of `a[10]`,
 /// `int a[1][2][3];` -> range of `a[1][2][3]`,
 /// `int *a = nullptr` -> range of `*a = nullptr`.
-/// FIMXE: \p Name must be a source range, e.g. for `operator+`.
+/// `int S::f(){}` -> range of `S::f()`.
+/// FIXME: \p Name must be a source range, e.g. for `operator+`.
 static SourceRange getDeclaratorRange(const SourceManager &SM, TypeLoc T,
   SourceLocation Name,
   SourceRange Initializer) {
   SourceLocation Start = GetStartLoc().Visit(T);
-  SourceLocation End = T.getSourceRange().getEnd();
+  SourceLocation End = T.getEndLoc();
   assert(End.isValid());
   if (Name.isValid()) {
 if (Start.isInvalid())
@@ -378,11 +417,9 @@
 
   /// Returns true if \p D is the last declarator in a chain and is thus
   /// reponsible for creating SimpleDeclaration for the whole chain.
-  template 
-  bool isResponsibleForCreatingDeclaration(const T *D) const {
-static_assert((std::is_base_of::value ||
-   std::is_base_of::value),
-  "only DeclaratorDecl and TypedefNameDecl are supported.");
+  bool isResponsibleForCreatingDeclaration(const Decl *D) const {
+assert((isa(D)) &&
+   "only DeclaratorDecl and TypedefNameDecl are supported.");
 
 const Decl *Next = D->getNextDeclInContext();
 
@@ -390,15 +427,14 @@
 if (Next == nullptr) {
   return true;
 }
-const auto *NextT = dyn_cast(Next);
 
 // Next sibling is not the same type, this one is responsible.
-if (NextT == nullptr) {
+if (D->getKind() != Next->getKind()) {
   return true;
 }
 // Next sibling doesn't begin at the same loc, it must be a different
 // declaration, so this declarator is responsible.
-if (NextT->getBeginLoc() != D->getBeginLoc()) {
+if (Next->getBeginLoc() != D->getBeginLoc()) {
   return true;
 }
 
@@ -1405,43 +1441,12 @@
   }
 
 private:
-  template  SourceLocation getQualifiedNameStart(T *D) {
-static_assert((std::is_base_of::value ||
-   std::is_base_of::value),
-  "only DeclaratorDecl and TypedefNameDecl are supported.");
-
-auto DN = D->getDeclName();
-bool IsAnonymous = DN.isIdentifier() && !DN.getAsIdentifierInfo();
-if (IsAnonymous)
-  return SourceLocation();
-
-if (const auto *DD = dyn_cast(D)) {
-  if (DD->getQualifierLoc()) {
-return DD->getQualifierLoc().getBeginLoc();
-  }
-}
-
-return D->getLocation();
-  }
-
-  SourceRange getInitializerRange(Decl *D) {
-if (auto *V = dyn_cast(D)) {
-  auto *I = V->getInit();
-  // Initializers in range-based-for are not part of the declarator
-  if (I && !V->isCXXForRangeDecl())
-return I->getSourceRange();
-}

[clang] 38bc006 - [SyntaxTree][NFC] Refactor function templates into functions taking base class

[Eduardo Caldas via cfe-commits]

Author: Eduardo Caldas
Date: 2020-08-28T12:19:38Z
New Revision: 38bc0060e60fef5395c23b8b75163e5bdee23af6

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

LOG: [SyntaxTree][NFC] Refactor function templates into functions taking base 
class

The refactored functions were
* `isReponsibleForCreatingDeclaration`
* `getQualifiedNameStart`

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

Added: 


Modified: 
clang/lib/Tooling/Syntax/BuildTree.cpp

Removed: 




diff  --git a/clang/lib/Tooling/Syntax/BuildTree.cpp 
b/clang/lib/Tooling/Syntax/BuildTree.cpp
index b07e9c3faf9d..2e9e74401e71 100644
--- a/clang/lib/Tooling/Syntax/BuildTree.cpp
+++ b/clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -197,18 +197,57 @@ static syntax::NodeKind getOperatorNodeKind(const 
CXXOperatorCallExpr &E) {
   llvm_unreachable("Unknown OverloadedOperatorKind enum");
 }
 
+/// Get the start of the qualified name. In the examples below it gives the
+/// location of the `^`:
+/// `int ^a;`
+/// `int ^a::S::f(){}`
+static SourceLocation getQualifiedNameStart(NamedDecl *D) {
+  assert((isa(D)) &&
+ "only DeclaratorDecl and TypedefNameDecl are supported.");
+
+  auto DN = D->getDeclName();
+  bool IsAnonymous = DN.isIdentifier() && !DN.getAsIdentifierInfo();
+  if (IsAnonymous)
+return SourceLocation();
+
+  if (const auto *DD = dyn_cast(D)) {
+if (DD->getQualifierLoc()) {
+  return DD->getQualifierLoc().getBeginLoc();
+}
+  }
+
+  return D->getLocation();
+}
+
+/// Gets the range of the initializer inside an init-declarator C++ [dcl.decl].
+/// `int a;` -> range of ``,
+/// `int *a = nullptr` -> range of `= nullptr`.
+/// `int a{}` -> range of `{}`.
+/// `int a()` -> range of `()`.
+static SourceRange getInitializerRange(Decl *D) {
+  if (auto *V = dyn_cast(D)) {
+auto *I = V->getInit();
+// Initializers in range-based-for are not part of the declarator
+if (I && !V->isCXXForRangeDecl())
+  return I->getSourceRange();
+  }
+
+  return SourceRange();
+}
+
 /// Gets the range of declarator as defined by the C++ grammar. E.g.
 /// `int a;` -> range of `a`,
 /// `int *a;` -> range of `*a`,
 /// `int a[10];` -> range of `a[10]`,
 /// `int a[1][2][3];` -> range of `a[1][2][3]`,
 /// `int *a = nullptr` -> range of `*a = nullptr`.
-/// FIMXE: \p Name must be a source range, e.g. for `operator+`.
+/// `int S::f(){}` -> range of `S::f()`.
+/// FIXME: \p Name must be a source range, e.g. for `operator+`.
 static SourceRange getDeclaratorRange(const SourceManager &SM, TypeLoc T,
   SourceLocation Name,
   SourceRange Initializer) {
   SourceLocation Start = GetStartLoc().Visit(T);
-  SourceLocation End = T.getSourceRange().getEnd();
+  SourceLocation End = T.getEndLoc();
   assert(End.isValid());
   if (Name.isValid()) {
 if (Start.isInvalid())
@@ -378,11 +417,9 @@ class syntax::TreeBuilder {
 
   /// Returns true if \p D is the last declarator in a chain and is thus
   /// reponsible for creating SimpleDeclaration for the whole chain.
-  template 
-  bool isResponsibleForCreatingDeclaration(const T *D) const {
-static_assert((std::is_base_of::value ||
-   std::is_base_of::value),
-  "only DeclaratorDecl and TypedefNameDecl are supported.");
+  bool isResponsibleForCreatingDeclaration(const Decl *D) const {
+assert((isa(D)) &&
+   "only DeclaratorDecl and TypedefNameDecl are supported.");
 
 const Decl *Next = D->getNextDeclInContext();
 
@@ -390,15 +427,14 @@ class syntax::TreeBuilder {
 if (Next == nullptr) {
   return true;
 }
-const auto *NextT = dyn_cast(Next);
 
 // Next sibling is not the same type, this one is responsible.
-if (NextT == nullptr) {
+if (D->getKind() != Next->getKind()) {
   return true;
 }
 // Next sibling doesn't begin at the same loc, it must be a 
diff erent
 // declaration, so this declarator is responsible.
-if (NextT->getBeginLoc() != D->getBeginLoc()) {
+if (Next->getBeginLoc() != D->getBeginLoc()) {
   return true;
 }
 
@@ -1405,43 +1441,12 @@ class BuildTreeVisitor : public 
RecursiveASTVisitor {
   }
 
 private:
-  template  SourceLocation getQualifiedNameStart(T *D) {
-static_assert((std::is_base_of::value ||
-   std::is_base_of::value),
-  "only DeclaratorDecl and TypedefNameDecl are supported.");
-
-auto DN = D->getDeclName();
-bool IsAnonymous = DN.isIdentifier() && !DN.getAsIdentifierInfo();
-if (IsAnonymous)
-  return SourceLocation();
-
-if (const auto *DD = dyn_cast(D)) {
-  if (DD->getQualifierLoc()) {
-return DD->getQualifierLoc().getBeginL

[clang] a146195 - [SyntaxTree] Add coverage for declarators and init-declarators

[Eduardo Caldas via cfe-commits]

Author: Eduardo Caldas
Date: 2020-08-28T12:19:38Z
New Revision: a1461953f4efe574e3fdecfbae68bd18707748fb

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

LOG: [SyntaxTree] Add coverage for declarators and init-declarators

Added: 


Modified: 
clang/lib/Tooling/Syntax/BuildTree.cpp
clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

Removed: 




diff  --git a/clang/lib/Tooling/Syntax/BuildTree.cpp 
b/clang/lib/Tooling/Syntax/BuildTree.cpp
index 2e9e74401e71..a9f326439a2a 100644
--- a/clang/lib/Tooling/Syntax/BuildTree.cpp
+++ b/clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -200,6 +200,7 @@ static syntax::NodeKind getOperatorNodeKind(const 
CXXOperatorCallExpr &E) {
 /// Get the start of the qualified name. In the examples below it gives the
 /// location of the `^`:
 /// `int ^a;`
+/// `int *^a;`
 /// `int ^a::S::f(){}`
 static SourceLocation getQualifiedNameStart(NamedDecl *D) {
   assert((isa(D)) &&
@@ -242,7 +243,7 @@ static SourceRange getInitializerRange(Decl *D) {
 /// `int a[1][2][3];` -> range of `a[1][2][3]`,
 /// `int *a = nullptr` -> range of `*a = nullptr`.
 /// `int S::f(){}` -> range of `S::f()`.
-/// FIXME: \p Name must be a source range, e.g. for `operator+`.
+/// FIXME: \p Name must be a source range.
 static SourceRange getDeclaratorRange(const SourceManager &SM, TypeLoc T,
   SourceLocation Name,
   SourceRange Initializer) {

diff  --git a/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp 
b/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
index a07187e22e93..aab20008a497 100644
--- a/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ b/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -3123,6 +3123,35 @@ SimpleDeclaration
 )txt"}));
 }
 
+TEST_P(SyntaxTreeTest, OutOfLineMemberFunctionDefinition) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct S {
+  void f();
+};
+[[void S::f(){}]]
+)cpp",
+  {R"txt(
+SimpleDeclaration
+|-'void'
+|-SimpleDeclarator Declarator
+| |-NestedNameSpecifier
+| | |-IdentifierNameSpecifier ListElement
+| | | `-'S'
+| | `-'::' ListDelimiter
+| |-'f'
+| `-ParametersAndQualifiers
+|   |-'(' OpenParen
+|   `-')' CloseParen
+`-CompoundStatement
+  |-'{' OpenParen
+  `-'}' CloseParen
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest, ConversionMemberFunction) {
   if (!GetParam().isCXX()) {
 return;
@@ -3792,6 +3821,53 @@ TranslationUnit Detached
 )txt"));
 }
 
+TEST_P(SyntaxTreeTest, InitDeclarator_Brace) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqual(
+  R"cpp(
+int a {};
+)cpp",
+  R"txt(
+TranslationUnit Detached
+`-SimpleDeclaration
+  |-'int'
+  |-SimpleDeclarator Declarator
+  | |-'a'
+  | `-UnknownExpression
+  |   `-UnknownExpression
+  | |-'{'
+  | `-'}'
+  `-';'
+)txt"));
+}
+
+TEST_P(SyntaxTreeTest, InitDeclarator_Paren) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct S {
+  S(int);
+};
+[[S s(1);]]
+)cpp",
+  {R"txt(
+SimpleDeclaration
+|-'S'
+|-SimpleDeclarator Declarator
+| `-UnknownExpression
+|   |-'s'
+|   |-'('
+|   |-IntegerLiteralExpression
+|   | `-'1' LiteralToken
+|   `-')'
+`-';'
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest, ArrayDeclarator_Simple) {
   EXPECT_TRUE(treeDumpEqual(
   R"cpp(



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

[PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:1755-1759
+  QualType FromTy = ArrayFrom->getElementType();
+  QualType ToTy = ArrayTo->getElementType();
+
+  FromRecordDecl = FromTy->getAsRecordDecl();
+  ToRecordDecl = ToTy->getAsRecordDecl();

labath wrote:
> shafik wrote:
> > martong wrote:
> > > labath wrote:
> > > > What about 2- or n-dimensional arrays?
> > > @labath, this is a very good question! And made me realize that D71378 is 
> > > fundamentally flawed (@shafik, please take no offense). Let me explain.
> > > 
> > > So, with D71378, we added the `if (ImportedOrErr) { ... }` block to 
> > > import definitions specifically of fields' Record types. But we forget to 
> > > handle arrays. Now we may forget to handle multidimensional arrays ... 
> > > and we may forget to handle other language constructs. So, we would 
> > > finally end up in re-implementing the logic of 
> > > `ASTNodeImporter::VisitFieldDecl`.
> > > 
> > > So all this should have been handled properly by the preceding import 
> > > call of the FieldDecl! Here
> > > ```
> > > 1735: ExpectedDecl ImportedOrErr = import(From);
> > > ```
> > > I have a suspicion that real reason why this import call fails in case of 
> > > the public ASTImporter::ImportDefinition() is that we fail to drive 
> > > through the import kind (`IDK_Everything`) during the import process.
> > > Below we set IDK_Everything and start a complete import process.
> > > ```
> > >   8784   if (auto *ToRecord = dyn_cast(To)) {
> > >   8785 if (!ToRecord->getDefinition()) {
> > >   8786   return Importer.ImportDefinition(   // 
> > > ASTNodeImporter::ImportDefinition !
> > >   8787   cast(FromDC), ToRecord,
> > >   8788   ASTNodeImporter::IDK_Everything);
> > >   8789 }
> > >   8790   }
> > > ```
> > > However, there may be many places where we fail to channel through that 
> > > we want to do a complete import. E.g here
> > > ```
> > > 1957   
> > > ImportDefinitionIfNeeded(Base1.getType()->getAsCXXRecordDecl()))
> > > ```
> > > we do another definition import and IDK_Everything is not set. So we may 
> > > have a wrong kind of import since the "minimal" flag is set.
> > > 
> > > The thing is, it is really confusing and error-prone to have both the 
> > > `ASTImporter::Minimal` flag and the `ImportDefinitionKind`. They seem to 
> > > be in contradiction to each other some times.
> > > I think we should get rid of the Minimal flag. And Kind should be either 
> > > a full completion (IDK_Everythink) or just a minimal (IDK_Basic). So, I'd 
> > > scrap the IDK_Default too. Alternatively we could have a Kind member in 
> > > AST**//Node//**Importer.
> > > I think we should go into this direction to avoid similar problems during 
> > > CodeGen in the future. WDYT?
> > No offense, you definitely understand the Importer better than I, so I 
> > value your input here. I don't believe that should have other fields where 
> > we could have a record that effects the layout but the concern is still 
> > valid and yes I did miss multi-dimensional arrays.
> > 
> > Your theory on `IDK_Everything` not be pushed through everywhere, I did a 
> > quick look and it seems pretty reasonable. 
> > 
> > I agree that the `ASTImporter::Minimal` flag and the `ImportDefinitionKind` 
> > seem to inconsistent or perhaps a work-around. That seems like a bigger 
> > change with a lot more impact beyond this fix but worth looking into longer 
> > term. 
> > 
> > If pushing through `IDK_Everything` everywhere fixes the underlying issue I 
> > am very happy to take that approach. If not we can discuss alternatives. 
> I've been looking at this code, but I'm still not very familiar with it, so 
> what I am asking may be obvious, but... What is the expected behavior for 
> non-minimal imports for code like this?
> ```
> struct A { ...};
> struct B { A* a; }; // Note the pointer
> ```
> Should importing B also import the definition of the A struct ? As I think 
> that should not happen in the minimal import... If we get rid of the minimal 
> flag, and rely solely on argument passing, we will need to be careful, as it 
> shouldn't be passed _everywhere_ (it should stop at pointers for instance). 
> But then it may not be possible to reproduce the current non-minimal import, 
> as it (I think) expects that A will be fully imported too...
> 
> > I don't believe that should have other fields where we could have a record 
> > that effects the layout
> This isn't exactly layout related, but there is the question of covariant 
> methods. If a method is covariant, then its return type must be complete. 
> Currently we handle the completion of these in LLDB, but that solution is a 
> bit hacky. It might be interesting if that could be handled by the ast 
> importer as well.
> What is the expected behavior for non-minimal imports for code like this?
In this case we import the definition of

[PATCH] D86780: Copy blocks in variadic methods

[酷酷的哀殿 via Phabricator via cfe-commits]

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

This commit will fix https://bugs.llvm.org/show_bug.cgi?id=46399.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86780

Files:
  clang/lib/Sema/SemaExprObjC.cpp


Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -1777,6 +1777,17 @@
 }
   }
 
+  for (unsigned i = NumNamedArgs, e = Args.size(); i < e; ++i) {
+if (Args[i]->isTypeDependent())
+  continue;
+// copy blocks [NSArray arrayWithObjects:^(){NSLog(@"blk0:%d", 
val);},^(){NSLog(@"blk1:%d", val);}, nil];
+if (Args[i]->getType()->isBlockPointerType()) {
+  ExprResult arg = Args[i];
+  maybeExtendBlockObject(arg);
+  Args[i] = arg.get();
+}
+  }
+
   DiagnoseSentinelCalls(Method, SelLoc, Args);
 
   // Do additional checkings on method.


Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -1777,6 +1777,17 @@
 }
   }
 
+  for (unsigned i = NumNamedArgs, e = Args.size(); i < e; ++i) {
+if (Args[i]->isTypeDependent())
+  continue;
+// copy blocks [NSArray arrayWithObjects:^(){NSLog(@"blk0:%d", val);},^(){NSLog(@"blk1:%d", val);}, nil];
+if (Args[i]->getType()->isBlockPointerType()) {
+  ExprResult arg = Args[i];
+  maybeExtendBlockObject(arg);
+  Args[i] = arg.get();
+}
+  }
+
   DiagnoseSentinelCalls(Method, SelLoc, Args);
 
   // Do additional checkings on method.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:896-898
+RawLexer.reset(new Lexer(SM.getLocForStartOfFile(LocInfo.first), LangOpts,
+ MB->getBufferStart(), MacroNameTokenPos,
+ MB->getBufferEnd()));

Szelethus wrote:
> steakhal wrote:
> > I'm always puzzled if I see a naked `new`.
> > Couldn't we use the assignment operator and `std::make_unique` here?
> Wait, isn't it naked if its //not// surrounded by smart pointer stuff? In any 
> case, explicit calls to operator `new` and `delete` are indeed discouraged by 
> the 
> [[http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r11-avoid-calling-new-and-delete-explicitly
>  | core guidelines]].
It is not enough to call a smart pointer's constructor with a result of a naked 
new. Because after the allocation of the object, the object's constructor 
itself could throw (well, not in LLVM :D) and this could happen before 
acquiring the ownership by the smart pointer, bumm, we have a leak.


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

https://reviews.llvm.org/D86135

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

[PATCH] D86743: [analyzer] Ignore VLASizeChecker case that could cause crash

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

balazske added a comment.

The fix is probably OK but I could not find out what causes the problem in this 
case and not in other (similar) ones.
Why is not possible to assume `SVB.evalEQ(State, DynSize, *ArraySizeNL)` to 
true:
DynSize: `extent_$1{e}`
*ArraySizeNL: `8 U64b`
The problem occurs likely not at the first iteration of the loop. Probably 
something is "messed up" in the state.




Comment at: clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp:304
+// See https://bugs.llvm.org/show_bug.cgi?id=47272.
+if (!State)
+  return;

If the previous assumption fails the assumptions made before it (about size of 
array dimensions) can be applied. Like at line 284 a transition should be 
added. At least if the current state indicates not a problem that makes this 
unnecessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86743

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

[PATCH] D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D86135#2235473 , @Szelethus wrote:

> In D86135#2233611 , @martong wrote:
>
>>> The fundamental problem is, we simply can't ask Preprocessor what a macro 
>>> expands into without hacking really hard.
>>
>> Can you summarize what is the exact problem (or give a link to a discussion, 
>> etc)? Is it an architectural problem in Clang itself?
>
> I phrased myself poorly. The Preprocessor can tell what a macro usage 
> immediately expands into (otherwise this project would have been practically 
> impossible to implement), what it struggles to get done is show what a macro 
> usage in the source code turns into in the preprocessed translation unit. As 
> is often the case, macros may be nested into one another:
>
>   #define MACRO2(ptr) ptr = 0
>   #define MACRO1(x) MACRO2(x)
>   
>   int *a = get();
>   MACRO1(a); // Step 1. Expand MACRO1.
>  //a.) Retrieve the tokens defined for MACRO1.
>  //b.) Resolve parameter x to argument a.
>  // Step 2. Expand MACRO2...
>   *a = 5;
>
> From this code snippet, should we be interested in what `MACRO1(a)` expands 
> into, `Preprocessor` can pretty much only deliver on point 1a.) with any 
> convenience. The problem here is that it was simply never engineered to be 
> used outside of the preprocessing stage, so much so, that by the time I 
> worked on this project about a decade after `Preprocessor`'s inception, I 
> still had to turn super trivial methods const. Other than that, the 
> discussions I linked in the summary is pretty much all I can offer.
>
>> Could we somehow refactor Clang and the Preprocessor to be usable for us? I 
>> mean LLVM and Clang has the mindset to build reusable components, the 
>> Preprocessor (and the Sema) should be one of them too, not just the AST.
>
> Working with the `Preprocessor` seems incredibly tedious and error prone -- 
> mind that this is almost literally the first thing we implemented in Clang, 
> and you can tell. Also, its performance critical, and any meaningful 
> performance impact may need strong justification. While it would be 
> beneficial to have the knowledge I'm crafting being integrated into the 
> actual class, its hard to justify all the effort it would take to do so, 
> especially now this project is so far along anyways. If I has to start from 
> scratch, I would try to explore other approaches, but might still end up just 
> doing what I did here.
>
> In D86135#2233695 , @xazax.hun wrote:
>
>> Is this related to https://bugs.llvm.org/show_bug.cgi?id=44493?
>
> To some extent, but this patch unfortunately doesn't solve that bug. The 
> problem I think is that named variadic macros aren't supported at all.

Thanks, for the detailed explanation, makes it easier to understand the reasons!




Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:1227
+  if (TheTok.getIdentifierInfo() == __VA_ARGS__II) {
+TStream.injectRange(PrevParamMap.at(__VA_ARGS__II));
+TStream.next(TheTok);

Why do we have to push back the tokens in case of __VA_ARGS? And what is in 
PrevParamMap here. Is it possible that `at` can fail here? Perhaps an example 
could make this hunk way easier to understand. To be honest, this hunk is a 
mystique for me in this form. 


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

https://reviews.llvm.org/D86135

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

[PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

[Pavel Labath via Phabricator via cfe-commits]

labath added a comment.

I'm going to respond to the rest of your (very insightful) comment later. So 
far, I'm just responding to this:

>>   This isn't exactly layout related, but there is the question of covariant 
>> methods. If a method is covariant, then its return type must be complete.
>
> We already import all the base classes of a derived class (at least in full 
> import). But, we do not import all the derived classes during the import of 
> the base. Is this what you do in LLDB to support covariant return types?

This situation is a bit tricky to explain as there are two independent class 
hierarchies that need to be considered. Let me start with an example:

  struct B1;
  struct B2;
  struct A1 { B1 *f(); };
  struct A2 { B2 *f(); };

This is a perfectly valid C++ snippet. In fact it could be extended to also 
implement `A1::f` and `A2::f` and call them and it would still not require the 
definitions for structs `B1` and `B2`. And I believe the ast importer will 
would not import their definitions even if they were available. It certainly 
wouldn't force them to be defined (`getExternalSource()->CompleteType(B1)`).

This changes if the methods become virtual. In this case, this code becomes 
valid only if `B2*` is convertible to `B1*` (i.e., `B2` is derived from `B1`). 
To know that, both `B1` and `B2` have to be complete. Regular clang parser will 
enforce that, and codegen will depend on it. However, the AST importer will not 
automatically import these classes. Lldb's code to do that is here 
https://github.com/llvm/llvm-project/blob/fdc6aea3fd822b639baaa5b666fdf7598d08c8de/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp#L994.

I don't know whether something like this would be in scope for the default ast 
importer, but it seemed useful to mention in the context of the present 
discussion.


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

https://reviews.llvm.org/D86660

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

[PATCH] D86782: [clang-format] Allow configuring list of macros that map to attributes

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: MyDeveloperDay, JakeMerdichAMD, sammccall, 
curdeius, jrtc27.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
arichardson requested review of this revision.

This adds a `AttributeMacros` configuration option that causes certain
identifiers to be parsed like a __attribute__((foo)) annotation.
This is motivated by our CHERI C/C++ fork which adds a __capability
qualifier for pointer/reference. Without this change clang-format parses
many type declarations as multiplications/bitwise-and instead.
I initially considered adding "__capability" as a new clang-format keyword,
but having a list of macros that should be treated as attributes is more
flexible since it can be used e.g. for static analyzer annotations or other 
language
extensions.

Example: std::vector -> std::vector

Depends on D86775  (to apply cleanly)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86782

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8030,7 +8030,20 @@
   verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  FormatStyle CustomQualifier = getLLVMStyle();
+  // Add indentifers that should not be parsed as a qualifier by default.
+  CustomQualifier.AttributeMacros.push_back("__my_qualifier");
+  CustomQualifier.AttributeMacros.push_back("_My_qualifier");
+  CustomQualifier.AttributeMacros.push_back("my_other_qualifier");
+  verifyFormat("vector parse_as_multiply;");
+  verifyFormat("vector v;", CustomQualifier);
+  verifyFormat("vector parse_as_multiply;");
+  verifyFormat("vector v;", CustomQualifier);
+  verifyFormat("vector parse_as_multiply;");
+  verifyFormat("vector v;", CustomQualifier);
   verifyFormat("vector v;");
+  verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("foo();");
   verifyFormat("foo();");
@@ -8073,10 +8086,23 @@
   verifyIndependentOfContext("MACRO(A *[[clang::attr]] a);");
   verifyIndependentOfContext("MACRO(A *__ptr32 a);");
   verifyIndependentOfContext("MACRO(A *__ptr64 a);");
+  verifyIndependentOfContext("MACRO(A *__capability);");
+  verifyIndependentOfContext("MACRO(A &__capability);");
+  verifyFormat("MACRO(A *__my_qualifier);"); // type declaration
+  verifyFormat("void f() { MACRO(A * __my_qualifier); }"); // multiplication
+  // If we add __my_qualifier to AttributeMacros it should always be parsed as
+  // a type declaration:
+  verifyFormat("MACRO(A *__my_qualifier);", CustomQualifier);
+  verifyFormat("void f() { MACRO(A *__my_qualifier); }", CustomQualifier);
+
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
   // FIXME: Is there a way to make this work?
   // verifyIndependentOfContext("MACRO(A *a);");
+  verifyFormat("MACRO(A &B);");
+  verifyFormat("MACRO(A *B);");
+  verifyFormat("void f() { MACRO(A * B); }");
+  verifyFormat("void f() { MACRO(A & B); }");
 
   verifyFormat("DatumHandle const *operator->() const { return input_; }");
   verifyFormat("return options != nullptr && operator==(*options);");
@@ -8126,10 +8152,47 @@
   verifyFormat("a __attribute__((unused))\n"
"aaa(int i);");
   FormatStyle AfterType = getLLVMStyle();
-  AfterType.AlwaysBreakAfterReturnType = FormatStyle::RTBS_AllDefinitions;
+  AfterType.AlwaysBreakAfterReturnType = FormatStyle::RTBS_All;
   verifyFormat("__attribute__((nodebug)) void\n"
"foo() {}\n",
AfterType);
+  verifyFormat("__unused void\n"
+   "foo() {}",
+   AfterType);
+
+  FormatStyle CustomAttrs = getLLVMStyle();
+  CustomAttrs.AttributeMacros.push_back("__unused");
+  CustomAttrs.AttributeMacros.push_back("__attr1");
+  CustomAttrs.AttributeMacros.push_back("__attr2");
+  CustomAttrs.AttributeMacros.push_back("no_underscore_attr");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  // Check that it is parsed as a multiplication without AttributeMacros and
+  // as a pointer qualifier when we add __user to AttributeMacros.
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;", CustomAttrs);
+  verifyFormat("vector v;", CustomAttrs);
+  verifyFormat("vector v;", CustomAttrs);
+  verifyFormat("vector v;", CustomAttrs);
+  verifyFormat("vector v;", CustomAttrs);
+  verifyFormat("vector v;", 

[PATCH] D86736: [analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a GDM trait

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> For any other loops, in order to know whether we should analyze another 
> iteration, among other things, we evaluate it's condition. Which is a problem 
> for ObjCForCollectionStmt, because it simply doesn't have one

Shouldn't we try to fix the ObjCForCollectionStmt in the AST rather? By adding 
the condition to that as a (sub)expression? There are already many 
discrepancies and inconsistencies between supposedly similar AST nodes, and 
people do fix them occasionally (e.g. D81787 ).
To be honest, it seems a bit off to store some parts of the liveness info in 
the GDM while other parts not in the GDM ...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86736

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

[PATCH] D86743: [analyzer] Ignore VLASizeChecker case that could cause crash

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers added a comment.

Thanks Balazs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86743

___
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

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

In D86671#2244236 , @dougpuob wrote:

> In D86671#2243788 , @Eugene.Zelenko 
> wrote:
>
>> It'll be good idea to add test case.
>
> Hi @Eugene.Zelenko,
> I have created a `readability-identifier-naming-hungarain-notion.cpp`  file 
> and several test cases for regression testing. Is it regression testing?
>
> Find it here (https://reviews.llvm.org/differential/changeset/?ref=2137882)

Yes, this is regression test. Please add newline at the end.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D86671

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

[PATCH] D86751: Add new warning for compound punctuation tokens that are split across macro expansions or split by whitespace.

[Samuel Benzaquen via Phabricator via cfe-commits]

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



Comment at: clang/test/Parser/compound-token-split.cpp:30
+
+[ // expected-warning-re ^}}'[' tokens introducing attribute appear in 
different source files}}
+#define LSQUARE

Does this count as a "different" source file?
Or is it just an implementation detail of how clang uses FileIDs ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86751

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

[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> Another solution for the problem is if the system calls are modeled in a way 
> that there is always a state split between error end non-error (we will have 
> a path where it is known that the specific variable can be only (for example) 
> NULL and this can be detected by other checkers).

I think this approach could be easily modeled in `StdLibraryFunctionCheckers` 
with the form of `Cases` (branches). E.g. 
https://github.com/llvm/llvm-project/blob/f20e6c7253859454c2f39adae19d80a31a0456a9/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp#L1139

Also, Comments about the branches: 
https://github.com/llvm/llvm-project/blob/f20e6c7253859454c2f39adae19d80a31a0456a9/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp#L13


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72705

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

[clang] f4257c5 - [SVE] Make ElementCount members private

[David Sherwood via cfe-commits]

Author: David Sherwood
Date: 2020-08-28T14:43:53+01:00
New Revision: f4257c5832aa51e960e7351929ca3d37031985b7

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

LOG: [SVE] Make ElementCount members private

This patch changes ElementCount so that the Min and Scalable
members are now private and can only be accessed via the get
functions getKnownMinValue() and isScalable(). In addition I've
added some other member functions for more commonly used operations.
Hopefully this makes the class more useful and will reduce the
need for calling getKnownMinValue().

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

Added: 


Modified: 
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CodeGenTypes.cpp
llvm/include/llvm/Analysis/TargetTransformInfo.h
llvm/include/llvm/Analysis/VectorUtils.h
llvm/include/llvm/CodeGen/ValueTypes.h
llvm/include/llvm/IR/DataLayout.h
llvm/include/llvm/IR/DerivedTypes.h
llvm/include/llvm/IR/Instructions.h
llvm/include/llvm/Support/MachineValueType.h
llvm/include/llvm/Support/TypeSize.h
llvm/lib/Analysis/InstructionSimplify.cpp
llvm/lib/Analysis/VFABIDemangling.cpp
llvm/lib/Analysis/ValueTracking.cpp
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
llvm/lib/CodeGen/CodeGenPrepare.cpp
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
llvm/lib/CodeGen/TargetLoweringBase.cpp
llvm/lib/CodeGen/ValueTypes.cpp
llvm/lib/IR/AsmWriter.cpp
llvm/lib/IR/ConstantFold.cpp
llvm/lib/IR/Constants.cpp
llvm/lib/IR/Core.cpp
llvm/lib/IR/DataLayout.cpp
llvm/lib/IR/Function.cpp
llvm/lib/IR/IRBuilder.cpp
llvm/lib/IR/Instructions.cpp
llvm/lib/IR/IntrinsicInst.cpp
llvm/lib/IR/Type.cpp
llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
llvm/lib/Transforms/Utils/FunctionComparator.cpp
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
llvm/lib/Transforms/Vectorize/VPlan.cpp
llvm/lib/Transforms/Vectorize/VPlan.h
llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
llvm/unittests/IR/VectorTypesTest.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 69899fd8e668..1192fbdc1c9d 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -8457,7 +8457,8 @@ Value 
*CodeGenFunction::EmitAArch64SVEBuiltinExpr(unsigned BuiltinID,
   case SVE::BI__builtin_sve_svlen_u64: {
 SVETypeFlags TF(Builtin->TypeModifier);
 auto VTy = cast(getSVEType(TF));
-auto NumEls = llvm::ConstantInt::get(Ty, VTy->getElementCount().Min);
+auto *NumEls =
+llvm::ConstantInt::get(Ty, VTy->getElementCount().getKnownMinValue());
 
 Function *F = CGM.getIntrinsic(Intrinsic::vscale, Ty);
 return Builder.CreateMul(NumEls, Builder.CreateCall(F));

diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index d90cffb4bb95..8a85a24910e4 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -726,7 +726,7 @@ llvm::DIType *CGDebugInfo::CreateType(const BuiltinType 
*BT) {
 {
   ASTContext::BuiltinVectorTypeInfo Info =
   CGM.getContext().getBuiltinVectorTypeInfo(BT);
-  unsigned NumElemsPerVG = (Info.EC.Min * Info.NumVectors) / 2;
+  unsigned NumElemsPerVG = (Info.EC.getKnownMinValue() * Info.NumVectors) 
/ 2;
 
   // Debuggers can't extract 1bit from a vector, so will display a
   // bitpattern for svbool_t instead.

diff  --git a/clang/lib/CodeGen/CodeGenTypes.cpp 
b/clang/lib/CodeGen/CodeGenTypes.cpp
index 9c072d416075..aede8a53ba90 100644
--- a/clang/lib/CodeGen/CodeGenTypes.cpp
+++ b/clang/lib/CodeGen/CodeGenTypes.cpp
@@ -586,7 +586,8 @@ llvm::Type *CodeGenTypes::ConvertType(QualType T) {
   ASTContext::BuiltinVectorTypeInfo Info =
   Context.getBuiltinVectorTypeInfo(cast(Ty));
   return llvm::ScalableVectorType::get(ConvertType(Info.ElementType),
-   Info.EC.Min * Info.NumVectors);
+   Info.EC.getKnownMinValue() *
+   Info.NumVectors);
 }
 case BuiltinType::Dependent:
 #define BUILTIN_TYPE(Id, SingletonId)

diff  --git a/llvm/include/llvm/Analysis/TargetTransformInfo.h 
b/llvm/include/llvm/Analysis/TargetTransformInfo.h
index a3e624842700..ffbec74c61d0 100644
--- a/llvm/include/llvm/Analysis/TargetTransformInfo.h
+++ b/llvm/include/llvm/Analysis/TargetTransformInfo.h
@@ -130,8 +130,8 @@ class IntrinsicCostAttributes {
   unsigned Fact

[PATCH] D86065: [SVE] Make ElementCount members private

[David Sherwood via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf4257c5832aa: [SVE] Make ElementCount members private 
(authored by david-arm).

Changed prior to commit:
  https://reviews.llvm.org/D86065?vs=288370&id=288594#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86065

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  llvm/include/llvm/Analysis/TargetTransformInfo.h
  llvm/include/llvm/Analysis/VectorUtils.h
  llvm/include/llvm/CodeGen/ValueTypes.h
  llvm/include/llvm/IR/DataLayout.h
  llvm/include/llvm/IR/DerivedTypes.h
  llvm/include/llvm/IR/Instructions.h
  llvm/include/llvm/Support/MachineValueType.h
  llvm/include/llvm/Support/TypeSize.h
  llvm/lib/Analysis/InstructionSimplify.cpp
  llvm/lib/Analysis/VFABIDemangling.cpp
  llvm/lib/Analysis/ValueTracking.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/CodeGen/TargetLoweringBase.cpp
  llvm/lib/CodeGen/ValueTypes.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/ConstantFold.cpp
  llvm/lib/IR/Constants.cpp
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/DataLayout.cpp
  llvm/lib/IR/Function.cpp
  llvm/lib/IR/IRBuilder.cpp
  llvm/lib/IR/Instructions.cpp
  llvm/lib/IR/IntrinsicInst.cpp
  llvm/lib/IR/Type.cpp
  llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
  llvm/lib/Transforms/Utils/FunctionComparator.cpp
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  llvm/lib/Transforms/Vectorize/VPlan.cpp
  llvm/lib/Transforms/Vectorize/VPlan.h
  llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
  llvm/unittests/IR/VectorTypesTest.cpp

Index: llvm/unittests/IR/VectorTypesTest.cpp
===
--- llvm/unittests/IR/VectorTypesTest.cpp
+++ llvm/unittests/IR/VectorTypesTest.cpp
@@ -119,8 +119,8 @@
   EXPECT_EQ(ConvTy->getElementType()->getScalarSizeInBits(), 64U);
 
   EltCnt = V8Int64Ty->getElementCount();
-  EXPECT_EQ(EltCnt.Min, 8U);
-  ASSERT_FALSE(EltCnt.Scalable);
+  EXPECT_EQ(EltCnt.getKnownMinValue(), 8U);
+  ASSERT_FALSE(EltCnt.isScalable());
 }
 
 TEST(VectorTypesTest, Scalable) {
@@ -215,8 +215,8 @@
   EXPECT_EQ(ConvTy->getElementType()->getScalarSizeInBits(), 64U);
 
   EltCnt = ScV8Int64Ty->getElementCount();
-  EXPECT_EQ(EltCnt.Min, 8U);
-  ASSERT_TRUE(EltCnt.Scalable);
+  EXPECT_EQ(EltCnt.getKnownMinValue(), 8U);
+  ASSERT_TRUE(EltCnt.isScalable());
 }
 
 TEST(VectorTypesTest, BaseVectorType) {
@@ -250,7 +250,7 @@
 // test I == J
 VectorType *VI = VTys[I];
 ElementCount ECI = VI->getElementCount();
-EXPECT_EQ(isa(VI), ECI.Scalable);
+EXPECT_EQ(isa(VI), ECI.isScalable());
 
 for (size_t J = I + 1, JEnd = VTys.size(); J < JEnd; ++J) {
   // test I < J
Index: llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
===
--- llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
+++ llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
@@ -71,8 +71,8 @@
 
   // Check fields inside llvm::ElementCount
   EltCnt = Vnx4i32.getVectorElementCount();
-  EXPECT_EQ(EltCnt.Min, 4U);
-  ASSERT_TRUE(EltCnt.Scalable);
+  EXPECT_EQ(EltCnt.getKnownMinValue(), 4U);
+  ASSERT_TRUE(EltCnt.isScalable());
 
   // Check that fixed-length vector types aren't scalable.
   EVT V8i32 = EVT::getVectorVT(Ctx, MVT::i32, 8);
@@ -82,8 +82,8 @@
 
   // Check that llvm::ElementCount works for fixed-length types.
   EltCnt = V8i32.getVectorElementCount();
-  EXPECT_EQ(EltCnt.Min, 8U);
-  ASSERT_FALSE(EltCnt.Scalable);
+  EXPECT_EQ(EltCnt.getKnownMinValue(), 8U);
+  ASSERT_FALSE(EltCnt.isScalable());
 }
 
 TEST(ScalableVectorMVTsTest, IRToVTTranslation) {
Index: llvm/lib/Transforms/Vectorize/VPlan.h
===
--- llvm/lib/Transforms/Vectorize/VPlan.h
+++ llvm/lib/Transforms/Vectorize/VPlan.h
@@ -151,14 +151,15 @@
   /// \return True if the map has a scalar entry for \p Key and \p Instance.
   bool hasScalarValue(Value *Key, const VPIteration &Instance) const {
 assert(Instance.Part < UF && "Queried Scalar Part is too large.");
-assert(Instance.Lane < VF.Min && "Queried Scalar Lane is too large.");
-assert(!VF.Scalable && "VF is assumed to be non scalable.");
+assert(Instance.Lane < VF.getKnownMinValue() &&
+   "Queried Scalar Lane is too large.");
+assert(!VF.isScalable() && "VF is assumed to be non scalable.");
 
 if (!hasAnyScalarValue(Key))
   return false;
 const ScalarParts &Entry = ScalarMapStorage.find(Key)->second;
 assert(Entry.size() == UF && "ScalarParts has wrong dimensions.");
-assert(Entry[Instance.Part].size() ==

[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D72705#2238487 , @Szelethus wrote:

> I debated this 
> 
>  report with @bruntib, depending from where you look at it, its either a 
> false positive or not -- in short, this is what happens:
>
>   char *p = /* some assumed to be valid c string */
>   end = strchr(p, '\n');
>   if (end - p > 4) { // <-- warning here on end's invalid use
>   }
>
> the guarded code wouldn't be executed, should `end` be zero. Now, if we 
> interpret the rule in a very literal sense, that
>
>> The successful completion or failure of each of the standard library 
>> functions listed in the following table shall be determined either by 
>> comparing the function’s return value with the value listed in the column 
>> labeled “Error Return” or by calling one of the library functions mentioned 
>> in the footnotes.
>
> then indeed, it doesn't compare the return value with the item in the table, 
> but it does compare it with superset of it. Now, we can either say that
>
> 1. A superset isn't good enough, because its a sign of code smell, or
> 2. Judge that the user probably did handle the error case, albeit in an 
> unconventional way, and report like these don't provide any value.
>
> I don't think we can realistically pursue point (2.). We would need to widen 
> the context (by going further up the AST with `Stmt::getParent()`) we're 
> considering for a potential check. However, forming a non-checking expression 
> that we don't immediately report can quickly get out of control:
>
>   char *p = /* some assumed to be valid c string */;
>   end = strchr(p, '\n');
>   int result = end - p;
>   if (result > 4) {
>   }
>
> You would be right to believe, without context from the code that is yet to 
> be executed, that the computation of `result` is invalid, because `end` is 
> unchecked. However, the code complies with the rule interpreted by (2.). This 
> seems to be a problem that can have infinite complexity without very severe 
> restriction on the analysis. Indeed, if we wander too far from the original 
> function call (by creating values that descend from the return value to be 
> checked), being sure that no checks happened (by point 2.) quickly becomes 
> unmanageable.
>
> So, for all practical purposes, we're locked to option (1.). However, the 
> warning message this way is imprecise -- we're **not** enforcing whether the 
> error value was checked for error, we're enforcing whether the return value 
> was checked against its respective failure value, and the message should 
> reflect that. In fact, the checker description, and some comments should be 
> clarified in this regard as well.
>
> I would imagine the pool of functions that deserve such incredibly strict 
> enforcement is rather small. Time management, maybe, but certainly not string 
> modeling functions, because problems related to them are solved by a 
> completely different checker infrastructure (`CStringChecker`), just as 
> stream modeling functions and memory management. Maybe we should only look 
> for statement-local bugs (basically a discarded return value) for functions 
> that don't demand this rigidness, but I suspect there is a clang-tidy check 
> already out there doing it. Seems like I got what @NoQ meant here.
>
> In D72705#2088319 , @NoQ wrote:
>
>> I still don't understand how do we enable this checker, i.e. for whom and 
>> how often. Is there anything we'll be able to turn on by default, like maybe 
>> warn on all functions that wear `__attribute__((warn_unused_result))`?
>
>
>
> ---
>
> I'm very happy we got another round of evaluations, we've gotten some 
> invaluable information out of it. For future patches, it would be great to 
> have your input on at least some of the reports as you're publishing them. 
> For the latest project, I think there is clear intent from the developer to 
> avoid null pointer misuses -- you should justify why a report is still 
> appropriate there.
>
> You mentioned in D72705#2187556  
> that on `duckdb` you stumbled across 1-2 false positives, but also that you 
> found 23 reports, and that you ran the checker on `vim`. This may be okay for 
> an alpha checker, or maybe not, if it points to a fundamental problem in your 
> approach. Please publish them before updating the patch. If all goes well and 
> those cases could be patched up later, here are the next important steps:
>
> - Have a warning message that precisely states what, and how the rule was 
> violated, even if its done with placeholders, even if you only 

[PATCH] D85031: [builtins] Unify the softfloat division implementation

[Anatoly Trosinenko via Phabricator via cfe-commits]

atrosinenko updated this revision to Diff 288599.
atrosinenko added a comment.

This update is expected to be completely NFC w.r.t. code behavior and 
significantly clarify the proof up to the end of half-width iterations.

Particularly, the reasoning about possible overflow of intermediate results 
turned out to be actually unclear/incorrect.

@sepavloff could you take a look on the new version in case it clarifies some 
of your questions? Another update for the second half of function may follow 
slightly later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85031

Files:
  compiler-rt/lib/builtins/divdf3.c
  compiler-rt/lib/builtins/divsf3.c
  compiler-rt/lib/builtins/divtf3.c
  compiler-rt/lib/builtins/fp_div_impl.inc
  compiler-rt/lib/builtins/fp_lib.h
  compiler-rt/lib/builtins/int_util.h
  compiler-rt/test/builtins/Unit/divdf3_test.c

Index: compiler-rt/test/builtins/Unit/divdf3_test.c
===
--- compiler-rt/test/builtins/Unit/divdf3_test.c
+++ compiler-rt/test/builtins/Unit/divdf3_test.c
@@ -92,5 +92,13 @@
 if (test__divdf3(0x1.0p+0, 0x1.0001p+0, UINT64_C(0x3fefffe0)))
   return 1;
 
+// some misc test cases obtained by fuzzing against h/w implementation
+if (test__divdf3(0x1.fdc239dd64735p-658, -0x1.fff9364c0843fp-948, UINT64_C(0xd20fdc8fc0ceffb1)))
+  return 1;
+if (test__divdf3(-0x1.78abb261d47c8p+794, 0x1.fb01d537cc5aep+266, UINT64_C(0xe0e7c6148ffc23e3)))
+  return 1;
+if (test__divdf3(-0x1.da7dfe6048b8bp-875, 0x1.ffc7ea3ff60a4p-610, UINT64_C(0xaf5dab1fe0269e2a)))
+  return 1;
+
 return 0;
 }
Index: compiler-rt/lib/builtins/int_util.h
===
--- compiler-rt/lib/builtins/int_util.h
+++ compiler-rt/lib/builtins/int_util.h
@@ -28,4 +28,20 @@
 #define COMPILE_TIME_ASSERT2(expr, cnt)\
   typedef char ct_assert_##cnt[(expr) ? 1 : -1] UNUSED
 
+// Force unrolling the code specified to be repeated N times.
+#define REPEAT_0_TIMES(code_to_repeat) /* do nothing */
+#define REPEAT_1_TIMES(code_to_repeat) code_to_repeat
+#define REPEAT_2_TIMES(code_to_repeat) \
+  REPEAT_1_TIMES(code_to_repeat)   \
+  code_to_repeat
+#define REPEAT_3_TIMES(code_to_repeat) \
+  REPEAT_2_TIMES(code_to_repeat)   \
+  code_to_repeat
+#define REPEAT_4_TIMES(code_to_repeat) \
+  REPEAT_3_TIMES(code_to_repeat)   \
+  code_to_repeat
+
+#define REPEAT_N_TIMES_(N, code_to_repeat) REPEAT_##N##_TIMES(code_to_repeat)
+#define REPEAT_N_TIMES(N, code_to_repeat) REPEAT_N_TIMES_(N, code_to_repeat)
+
 #endif // INT_UTIL_H
Index: compiler-rt/lib/builtins/fp_lib.h
===
--- compiler-rt/lib/builtins/fp_lib.h
+++ compiler-rt/lib/builtins/fp_lib.h
@@ -40,9 +40,12 @@
 
 #if defined SINGLE_PRECISION
 
+typedef uint16_t half_rep_t;
 typedef uint32_t rep_t;
+typedef uint64_t twice_rep_t;
 typedef int32_t srep_t;
 typedef float fp_t;
+#define HALF_REP_C UINT16_C
 #define REP_C UINT32_C
 #define significandBits 23
 
@@ -58,9 +61,11 @@
 
 #elif defined DOUBLE_PRECISION
 
+typedef uint32_t half_rep_t;
 typedef uint64_t rep_t;
 typedef int64_t srep_t;
 typedef double fp_t;
+#define HALF_REP_C UINT32_C
 #define REP_C UINT64_C
 #define significandBits 52
 
@@ -102,9 +107,11 @@
 #elif defined QUAD_PRECISION
 #if __LDBL_MANT_DIG__ == 113 && defined(__SIZEOF_INT128__)
 #define CRT_LDBL_128BIT
+typedef uint64_t half_rep_t;
 typedef __uint128_t rep_t;
 typedef __int128_t srep_t;
 typedef long double fp_t;
+#define HALF_REP_C UINT64_C
 #define REP_C (__uint128_t)
 // Note: Since there is no explicit way to tell compiler the constant is a
 // 128-bit integer, we let the constant be casted to 128-bit integer
Index: compiler-rt/lib/builtins/fp_div_impl.inc
===
--- /dev/null
+++ compiler-rt/lib/builtins/fp_div_impl.inc
@@ -0,0 +1,378 @@
+//===-- fp_div_impl.inc - Floating point division -*- 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 implements soft-float division with the IEEE-754 default
+// rounding (to nearest, ties to even).
+//
+//===--===//
+
+#include "fp_lib.h"
+
+// The __divXf3__ function implements Newton-Raphson floating point divis

[PATCH] D86743: [analyzer] Ignore VLASizeChecker case that could cause crash

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

balazske added a comment.

I do not know how these changes can appear:
F12796482: Screenshot from 2020-08-28 16-06-02.png 

The checker makes only 2 assumptions, about the array dimension being positive 
and about the size of the array and the extent. (This state transition happens 
when https://reviews.llvm.org/D81061 is applied, without it the state may look 
even worse.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86743

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

[PATCH] D72932: [ARM] Follow AACPS standard for volatile bit-fields access width

[Ties Stuij via Phabricator via cfe-commits]

stuij added a comment.

@ostannard: pinging on behalf of @dnsampaio. The changes still apply cleanly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72932

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

[PATCH] D86721: [clang-format] Parse double-square attributes as pointer qualifiers

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:8071
   verifyIndependentOfContext("MACRO(A *__attribute((foo)) a);");
+  verifyIndependentOfContext("MACRO(A *[[clang::attr]] a);");
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");

Can you also add a test case for an attribute with an argument list, e.g.,
```
MACRO(A *[[clang::attr("foobar")]] a);
```



Comment at: clang/unittests/Format/FormatTest.cpp:8141
   verifyFormat("x = (foo *_Nonnull)*v;");
+  verifyFormat("x = (foo *[[clang::attr]])*v;");
 

Same for the casting case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86721

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

[PATCH] D86531: [analyzer][StdLibraryFunctionsChecker][NFC] Use Optionals throughout the summary API

[Gabor Marton via Phabricator via cfe-commits]

martong marked 3 inline comments as done.
martong added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86531

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

[PATCH] D84603: Thread safety analysis: More consistent warning message

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Analysis/ThreadSafety.cpp:1644
 if (!LDat) {
-  Analyzer->Handler.handleMutexNotHeld("", D, POK, Cp.toString(),
-   LK_Shared, Loc);
+  Analyzer->Handler.handleMutexNotHeld("negative capability", D, POK,
+   Cp.toString(), LK_Shared, Loc);

aaronpuchert wrote:
> aaron.ballman wrote:
> > aaronpuchert wrote:
> > > aaron.ballman wrote:
> > > > aaronpuchert wrote:
> > > > > aaron.ballman wrote:
> > > > > > It's a bit odd that we aren't using `DiagKind` as below, I assume 
> > > > > > that's because this is a negative test and the others are positive 
> > > > > > tests, but doesn't this introduce a terminology difference where a 
> > > > > > positive failure may call it a mutex and a negative failure may 
> > > > > > call it a negative capability? Should this be hooked in to 
> > > > > > `ClassifyDiagnostic()` (perhaps we need a 
> > > > > > `ClassifyNegativeDiagnostic()`?
> > > > > My thinking was that whatever the positive capability is called, we 
> > > > > should only talk about a "negative capability" instead of a "negative 
> > > > > mutex" or a "negative role". Also because not holding a capability is 
> > > > > in some way its own kind of capability.
> > > > I may still be confused or thinking of this differently, but I would 
> > > > assume that a negative mutex would be a mutex that's explicitly not 
> > > > held, which you may want to ensure on a function boundary to avoid 
> > > > deadlock. From that, I'd have guessed we would want the diagnostic to 
> > > > read `cannot call function 'bar' while mutex 'mu' is held` or `calling 
> > > > function 'bar' requires mutex 'mu' to not be held` because that's more 
> > > > clear than talking about negative capabilities (when the user is 
> > > > thinking in terms of mutexes which are or aren't held).
> > > Now I get it. I don't see an issue with that, but we need to distinguish 
> > > between `EXCLUDES(mu)` and `REQUIRES(!mu)`. The former will produce 
> > > "cannot call function 'bar' while mutex 'mu' is held" and we probably 
> > > want the latter to produce a different warning message.
> > > 
> > > Now one argument for the existing scheme remains that with 
> > > `-Wthread-safety-negative`, if you see a warning like "acquiring mutex 
> > > 'mu' requires negative capability '!mu'" on a lock operation, you know 
> > > you can fix that by adding `REQUIRES(!mu)` to your function.
> > > 
> > > If we say "warning: acquiring mutex 'mu' requires mutex 'mu' not to be 
> > > held" it might not be as clear what we want.
> > > Now I get it. I don't see an issue with that, but we need to distinguish 
> > > between EXCLUDES(mu) and REQUIRES(!mu). The former will produce "cannot 
> > > call function 'bar' while mutex 'mu' is held" and we probably want the 
> > > latter to produce a different warning message.
> > 
> > Ahhh, that's a good point.
> > 
> > > Now one argument for the existing scheme remains that with 
> > > -Wthread-safety-negative, if you see a warning like "acquiring mutex 'mu' 
> > > requires negative capability '!mu'" on a lock operation, you know you can 
> > > fix that by adding REQUIRES(!mu) to your function.
> > >
> > > If we say "warning: acquiring mutex 'mu' requires mutex 'mu' not to be 
> > > held" it might not be as clear what we want.
> > 
> > Hm, that's a good point as well.
> > 
> > Now that I understand the situation a bit better, I will be happy with 
> > either route, so I leave the decision in your capable hands. (If we get it 
> > wrong, we can always change the diagnostics later.) Do you have a preferred 
> > approach?
> There are basically two warnings related to negative capabilities: one is 
> about acquiring a capability without holding a negative capability, the other 
> about calling a function without holding a negative capability. Negative 
> capabilities can't be acquired or released, nor do they protect anything.
> 
> The other warning message also says "negative capability '!mu'", but mentions 
> the original capability kind earlier:
> 
> ```
> def warn_acquire_requires_negative_cap : Warning<
>   "acquiring %0 '%1' requires negative capability '%2'">,
> ```
> 
> I think that makes sense because there is a relation between the "positive" 
> capability of whatever kind that we want to acquire and the negative 
> capability that we need. The situation here is different: I just have some 
> `CallExpr` to a function with `REQUIRES(!...)`.
> 
> For that we have two different warnings, depending on whether we know the 
> capability to be held or not:
> 
> ```
> void foo() REQUIRES(!mu);
> void bar() REQUIRES(!mu) {
>   mu.Lock();
>   foo();// warning: cannot call function 'foo' while mutex 'mu' is 
> held
>   mu.Unlock();
> }
> void baz() {
>   foo();// warning: calling function 'foo' requires holding [negative 
> capability] '!mu'

[PATCH] D67113: ICK_Function_Conversion is a third kind conversion

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D67113#2243451 , @aaronpuchert 
wrote:

> Maybe this is to trivial for a review. The comment on 
> `StandardConversionSequence::Third` in clang/Sema/Overload.h says
>
>> The third conversion can be a qualification conversion or a function 
>> conversion.

There may be a deeper issue here in that there are four standard conversion 
sequences, not three: http://eel.is/c++draft/conv#1, but otherwise this change 
seems correct to me (function conversion is certainly not in the first group).

Do you have a test case which can exercise this change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67113

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

[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

[Tibor Brunner via Phabricator via cfe-commits]

bruntib commandeered this revision.
bruntib added a reviewer: NorenaLeonetti.
bruntib added a comment.
Herald added subscribers: martong, steakhal, jfb.

I'll rebase this patch and continue its development.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D33825

___
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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h:44
   llvm::Optional
-  GetDeclFailureInfo(const StringRef &TypeName, const NamedDecl *Decl,
+  getDeclFailureInfo(const StringRef &TypeName, const NamedDecl *Decl,
  const SourceManager &SM) const override;

Given that the other functions in the class use the wrong style of casing, we 
should probably leave this declaration alone so it doesn't become locally 
inconsistent.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:428
 
   case IdentifierNamingCheck::CT_HungarianNotation: {
 const NamedDecl *pNamedDecl = dyn_cast(pDecl);

I feel like I must be missing something because I don't have this enum in my 
copy of ToT despite pulling this morning. Is the patch missing some content?



Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h:69
   llvm::Optional
-  GetDeclFailureInfo(const StringRef &Type, const NamedDecl *Decl,
+  getDeclFailureInfo(const StringRef &Type, const NamedDecl *Decl,
  const SourceManager &SM) const override;

Same here.



Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h:127
   virtual llvm::Optional
-  GetDeclFailureInfo(const StringRef &Type, const NamedDecl *Decl,
+  getDeclFailureInfo(const StringRef &Type, const NamedDecl *Decl,
  const SourceManager &SM) const = 0;

And same here.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D86671

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

[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

[Tibor Brunner via Phabricator via cfe-commits]

bruntib updated this revision to Diff 288606.
bruntib added a comment.
Herald added a subscriber: mgehre.

I rebased the patch so it compiles with master version of LLVM/Clang. I did no 
other change, so I would like if this patch would be committed on behalf of 
@NorenaLeonetti if the patch is acceptable.
I would kindly ask the reviewers to give some comments if any additional 
modification is needed. I run this checker on DuckDB project and this checker 
gave two reports on functions which shouldn't be used as signal handler:

duckdb-0.2.0/third_party/sqlsmith/sqlsmith.cc:38:17: do not use C++ constructs 
in signal handlers [cert-msc54-cpp]
https://github.com/cwida/duckdb/blob/master/third_party/sqlsmith/sqlsmith.cc#L38

duckdb-0.2.0/tools/shell/shell.c:8828:13: signal handlers must be 'extern "C"' 
[cert-msc54-cpp]
https://github.com/cwida/duckdb/blob/master/tools/shell/shell.c#L8828

I haven't found other C++ projects which use signals. However, this checker 
reports on the non-compliant code fragments of the corresponding SEI-CERT rule: 
https://wiki.sei.cmu.edu/confluence/display/cplusplus/MSC54-CPP.+A+signal+handler+must+be+a+plain+old+function.
 The two findings above also seem to be true positive.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D33825

Files:
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/clang-tidy/cert/CMakeLists.txt
  clang-tools-extra/clang-tidy/cert/SignalHandlerMustBePlainOldFunctionCheck.cpp
  clang-tools-extra/clang-tidy/cert/SignalHandlerMustBePlainOldFunctionCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-msc54-cpp.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cert-signal-handler-must-be-plain-old-function.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cert-signal-handler-must-be-plain-old-function.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cert-signal-handler-must-be-plain-old-function.cpp
@@ -0,0 +1,133 @@
+// RUN: %check_clang_tidy %s cert-msc54-cpp %t -- -- -std=c++11
+
+namespace std {
+extern "C" using signalhandler = void(int);
+int signal(int sig, signalhandler *func);
+}
+
+#define SIG_ERR -1
+#define SIGTERM 15
+
+static void sig_handler(int sig) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: signal handlers must be 'extern "C"' [cert-msc54-cpp]
+// CHECK-MESSAGES: :[[@LINE+3]]:39: note: given as a signal parameter here
+
+void install_signal_handler() {
+  if (SIG_ERR == std::signal(SIGTERM, sig_handler))
+return;
+}
+
+extern "C" void cpp_signal_handler(int sig) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not use C++ constructs in signal handlers [cert-msc54-cpp]
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: note: C++ construct used here
+  throw "error message";
+}
+
+void install_cpp_signal_handler() {
+  if (SIG_ERR == std::signal(SIGTERM, cpp_signal_handler))
+return;
+}
+
+void indirect_recursion();
+
+void cpp_like(){
+  try {
+cpp_signal_handler(SIG_ERR);
+  } catch(...) {
+// handle error
+  }
+}
+
+void no_body();
+
+void recursive_function() {
+  indirect_recursion();
+  cpp_like();
+  no_body();
+  recursive_function();
+}
+
+void indirect_recursion() {
+  recursive_function();
+}
+
+extern "C" void signal_handler_with_cpp_function_call(int sig) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not call functions with C++ constructs in signal handlers [cert-msc54-cpp]
+  // CHECK-MESSAGES: :[[@LINE-11]]:3: note: function called here
+  // CHECK-MESSAGES: :[[@LINE-23]]:3: note: C++ construct used here
+  recursive_function();
+}
+
+void install_signal_handler_with_cpp_function_call() {
+  if (SIG_ERR == std::signal(SIGTERM, signal_handler_with_cpp_function_call))
+return;
+}
+
+class TestClass {
+public:
+  static void static_function() {}
+};
+
+extern "C" void signal_handler_with_cpp_static_method_call(int sig) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not call functions with C++ constructs in signal handlers [cert-msc54-cpp]
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: note: function called here
+  TestClass::static_function();
+}
+
+void install_signal_handler_with_cpp_static_method_call() {
+  if (SIG_ERR == std::signal(SIGTERM, signal_handler_with_cpp_static_method_call))
+return;
+}
+
+
+// Something that does not trigger the check:
+
+extern "C" void c_sig_handler(int sig) {
+#define cbrt(X) _Generic((X), long double \
+ : cbrtl, default \
+ : cbrt)(X)
+  auto char c = '1';
+  extern _Thread_local double _Complex d;
+  static const unsigned long int i = sizeof(float);
+  void f();
+  volatile struct c_struct {
+enum e {};
+union u;
+  };
+  typedef bool boolean;
+label:
+  switch (c) {
+  case ('1'):
+break;
+  default:
+d = 1.2;
+  }
+

[PATCH] D86531: [analyzer][StdLibraryFunctionsChecker][NFC] Use Optionals throughout the summary API

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

balazske added inline comments.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1006
+return IntRangeVector{std::pair{b, *e}};
+  return IntRangeVector{};
+}

This return of empty vector and possibility of adding empty vector to range 
constraint is a new thing, probably it is better to check at create of range 
constraint (or other place) for empty range (in this case the summary could be 
made invalid)? But this occurs probably never because the matching type (of the 
max value) should be used at the same function and if the type is there the max 
value should be too.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1321
 
-Optional Mode_tTy = lookupType("mode_t", ACtx);
+Optional Mode_tTy = lookupTy("mode_t");
 

martong wrote:
> balazske wrote:
> > It is better to get every type at the start before adding functions, or 
> > group the functions and get some types at the start of these groups but 
> > mark the groups at least with comments.
> Well, with looked-up types I followed the usual convention to define a 
> variable right before using it. This means that we lookup a type just before 
> we try to add the function which first uses that type.
> 
> However, builtin types are defined at the beginning, because they are used 
> very often.
I still like it better if all the type variables are created at one place (can 
be more easy to maintain if order changes and we have one block of types and 
one of functions) but this is no reason to block this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86531

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

[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

[Tibor Brunner via Phabricator via cfe-commits]

bruntib updated this revision to Diff 288617.
bruntib added a comment.

Update lincense.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D33825

Files:
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/clang-tidy/cert/CMakeLists.txt
  clang-tools-extra/clang-tidy/cert/SignalHandlerMustBePlainOldFunctionCheck.cpp
  clang-tools-extra/clang-tidy/cert/SignalHandlerMustBePlainOldFunctionCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-msc54-cpp.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cert-signal-handler-must-be-plain-old-function.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cert-signal-handler-must-be-plain-old-function.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cert-signal-handler-must-be-plain-old-function.cpp
@@ -0,0 +1,133 @@
+// RUN: %check_clang_tidy %s cert-msc54-cpp %t -- -- -std=c++11
+
+namespace std {
+extern "C" using signalhandler = void(int);
+int signal(int sig, signalhandler *func);
+}
+
+#define SIG_ERR -1
+#define SIGTERM 15
+
+static void sig_handler(int sig) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: signal handlers must be 'extern "C"' [cert-msc54-cpp]
+// CHECK-MESSAGES: :[[@LINE+3]]:39: note: given as a signal parameter here
+
+void install_signal_handler() {
+  if (SIG_ERR == std::signal(SIGTERM, sig_handler))
+return;
+}
+
+extern "C" void cpp_signal_handler(int sig) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not use C++ constructs in signal handlers [cert-msc54-cpp]
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: note: C++ construct used here
+  throw "error message";
+}
+
+void install_cpp_signal_handler() {
+  if (SIG_ERR == std::signal(SIGTERM, cpp_signal_handler))
+return;
+}
+
+void indirect_recursion();
+
+void cpp_like(){
+  try {
+cpp_signal_handler(SIG_ERR);
+  } catch(...) {
+// handle error
+  }
+}
+
+void no_body();
+
+void recursive_function() {
+  indirect_recursion();
+  cpp_like();
+  no_body();
+  recursive_function();
+}
+
+void indirect_recursion() {
+  recursive_function();
+}
+
+extern "C" void signal_handler_with_cpp_function_call(int sig) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not call functions with C++ constructs in signal handlers [cert-msc54-cpp]
+  // CHECK-MESSAGES: :[[@LINE-11]]:3: note: function called here
+  // CHECK-MESSAGES: :[[@LINE-23]]:3: note: C++ construct used here
+  recursive_function();
+}
+
+void install_signal_handler_with_cpp_function_call() {
+  if (SIG_ERR == std::signal(SIGTERM, signal_handler_with_cpp_function_call))
+return;
+}
+
+class TestClass {
+public:
+  static void static_function() {}
+};
+
+extern "C" void signal_handler_with_cpp_static_method_call(int sig) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not call functions with C++ constructs in signal handlers [cert-msc54-cpp]
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: note: function called here
+  TestClass::static_function();
+}
+
+void install_signal_handler_with_cpp_static_method_call() {
+  if (SIG_ERR == std::signal(SIGTERM, signal_handler_with_cpp_static_method_call))
+return;
+}
+
+
+// Something that does not trigger the check:
+
+extern "C" void c_sig_handler(int sig) {
+#define cbrt(X) _Generic((X), long double \
+ : cbrtl, default \
+ : cbrt)(X)
+  auto char c = '1';
+  extern _Thread_local double _Complex d;
+  static const unsigned long int i = sizeof(float);
+  void f();
+  volatile struct c_struct {
+enum e {};
+union u;
+  };
+  typedef bool boolean;
+label:
+  switch (c) {
+  case ('1'):
+break;
+  default:
+d = 1.2;
+  }
+  goto label;
+  for (; i < 42;) {
+if (d == 1.2)
+  continue;
+else
+  return;
+  }
+  do {
+_Atomic int v = _Alignof(char);
+_Static_assert(42 == 42, "True");
+  } while (c == 42);
+}
+
+void install_c_sig_handler() {
+  if (SIG_ERR == std::signal(SIGTERM, c_sig_handler)) {
+// Handle error
+  }
+}
+
+extern "C" void signal_handler_with_function_pointer(int sig) {
+  void (*funp) (void);
+  funp = &cpp_like;
+  funp();
+}
+
+void install_signal_handler_with_function_pointer() {
+  if (SIG_ERR == std::signal(SIGTERM, signal_handler_with_function_pointer))
+return;
+}
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
@@ -111,6 +111,7 @@
`cert-mem57-cpp `_,
`cert-msc50-cpp `_,
`cert-msc51-cpp `_,
+   `cert-msc54-cpp `_,
`cert-oop57-cpp `_,
`cert-oop58-cpp `_,
`clang-analyzer-core.DynamicTypePropagation `_,
Index: clang-tools-extra/docs/clang-tidy/checks/cert-msc54-cpp.rst
==

[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

[JF Bastien via Phabricator via cfe-commits]

jfb requested changes to this revision.
jfb added a comment.
This revision now requires changes to proceed.
Herald added a subscriber: dexonsmith.

Please consider these changes, and whether this is relevant as implemented: 
http://wg21.link/p0270




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cert-signal-handler-must-be-plain-old-function.cpp:22
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not use C++ constructs in 
signal handlers [cert-msc54-cpp]
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: note: C++ construct used here
+  throw "error message";

"C++ construct" isn't particularly useful. Here it needs to call out throwing 
exceptions.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cert-signal-handler-must-be-plain-old-function.cpp:57
+  // CHECK-MESSAGES: :[[@LINE-11]]:3: note: function called here
+  // CHECK-MESSAGES: :[[@LINE-23]]:3: note: C++ construct used here
+  recursive_function();

Here too, I have no idea what this means given just this message.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cert-signal-handler-must-be-plain-old-function.cpp:73
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not call functions with C++ 
constructs in signal handlers [cert-msc54-cpp]
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: note: function called here
+  TestClass::static_function();

This is also very confusing.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D33825

___
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

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:428
 
   case IdentifierNamingCheck::CT_HungarianNotation: {
 const NamedDecl *pNamedDecl = dyn_cast(pDecl);

aaron.ballman wrote:
> I feel like I must be missing something because I don't have this enum in my 
> copy of ToT despite pulling this morning. Is the patch missing some content?
I feel like this is an incremental diff based on the first version of this 
patch rather than a diff from trunk. 


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D86671

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

[PATCH] D86065: [SVE] Make ElementCount members private

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm added a comment.

In D86065#2241146 , @david-arm wrote:

> Hi @ctetreau, ok for now I'm going to completely remove the operators and 
> revert the code using those operators to how it was before. I'm not sure what 
> you mean about the predicate functions so I've left those for now, since they 
> aren't needed for this patch. The purpose of this patch was originally 
> supposed to be mechanical anyway - just making members private. I only added 
> the operators as an after-thought really, just to be consistent with how 
> TypeSize dealt with the identical problem. For what it's worth, I believe 
> that GCC solved this exact same problem by adding two types of comparison 
> functions - one set that absolutely wanted an answer to ">,<,>=,<=" and 
> asserted if it wasn't known at compile time, and another set of comparison 
> functions that returned an additional boolean value indicating whether the 
> answer was known or not. Perhaps my knowledge is out of date, but I believe 
> this was the accepted solution and seemed to work well.

FWIW, the GCC scheme is to have one set of functions `maybe_` that are 
true if a condition *might* hold (i.e. would hold for one possible value of the 
runtime indeterminates), and another set of functions `known_` 
(originally `must_`) that are true if a condition *always* holds (i.e. 
would hold for all possible values of the runtime indeterminates).  `known_le` 
is a partial order but `maybe_le` is not (because it isn't antisymmetric).  
Having both is redundant with `!`, since e.g. `known_le` is the opposite of 
`maybe_gt`, but it seemed more readable to allow every condition to be 
expressed positively.

Like you say, there is also a test for whether two values are ordered by 
`known_le`, and there are also some operations like `ordered_min` and 
`ordered_max` that assert if the values aren't ordered by `known_le`.

[Sorry for the post-commit comment, but it's related to something that wasn't 
part of the commit.]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86065

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

[PATCH] D85031: [builtins] Unify the softfloat division implementation

[Anatoly Trosinenko via Phabricator via cfe-commits]

atrosinenko updated this revision to Diff 288623.
atrosinenko added a comment.

Add some other explanations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85031

Files:
  compiler-rt/lib/builtins/divdf3.c
  compiler-rt/lib/builtins/divsf3.c
  compiler-rt/lib/builtins/divtf3.c
  compiler-rt/lib/builtins/fp_div_impl.inc
  compiler-rt/lib/builtins/fp_lib.h
  compiler-rt/lib/builtins/int_util.h
  compiler-rt/test/builtins/Unit/divdf3_test.c

Index: compiler-rt/test/builtins/Unit/divdf3_test.c
===
--- compiler-rt/test/builtins/Unit/divdf3_test.c
+++ compiler-rt/test/builtins/Unit/divdf3_test.c
@@ -92,5 +92,13 @@
 if (test__divdf3(0x1.0p+0, 0x1.0001p+0, UINT64_C(0x3fefffe0)))
   return 1;
 
+// some misc test cases obtained by fuzzing against h/w implementation
+if (test__divdf3(0x1.fdc239dd64735p-658, -0x1.fff9364c0843fp-948, UINT64_C(0xd20fdc8fc0ceffb1)))
+  return 1;
+if (test__divdf3(-0x1.78abb261d47c8p+794, 0x1.fb01d537cc5aep+266, UINT64_C(0xe0e7c6148ffc23e3)))
+  return 1;
+if (test__divdf3(-0x1.da7dfe6048b8bp-875, 0x1.ffc7ea3ff60a4p-610, UINT64_C(0xaf5dab1fe0269e2a)))
+  return 1;
+
 return 0;
 }
Index: compiler-rt/lib/builtins/int_util.h
===
--- compiler-rt/lib/builtins/int_util.h
+++ compiler-rt/lib/builtins/int_util.h
@@ -28,4 +28,20 @@
 #define COMPILE_TIME_ASSERT2(expr, cnt)\
   typedef char ct_assert_##cnt[(expr) ? 1 : -1] UNUSED
 
+// Force unrolling the code specified to be repeated N times.
+#define REPEAT_0_TIMES(code_to_repeat) /* do nothing */
+#define REPEAT_1_TIMES(code_to_repeat) code_to_repeat
+#define REPEAT_2_TIMES(code_to_repeat) \
+  REPEAT_1_TIMES(code_to_repeat)   \
+  code_to_repeat
+#define REPEAT_3_TIMES(code_to_repeat) \
+  REPEAT_2_TIMES(code_to_repeat)   \
+  code_to_repeat
+#define REPEAT_4_TIMES(code_to_repeat) \
+  REPEAT_3_TIMES(code_to_repeat)   \
+  code_to_repeat
+
+#define REPEAT_N_TIMES_(N, code_to_repeat) REPEAT_##N##_TIMES(code_to_repeat)
+#define REPEAT_N_TIMES(N, code_to_repeat) REPEAT_N_TIMES_(N, code_to_repeat)
+
 #endif // INT_UTIL_H
Index: compiler-rt/lib/builtins/fp_lib.h
===
--- compiler-rt/lib/builtins/fp_lib.h
+++ compiler-rt/lib/builtins/fp_lib.h
@@ -40,9 +40,12 @@
 
 #if defined SINGLE_PRECISION
 
+typedef uint16_t half_rep_t;
 typedef uint32_t rep_t;
+typedef uint64_t twice_rep_t;
 typedef int32_t srep_t;
 typedef float fp_t;
+#define HALF_REP_C UINT16_C
 #define REP_C UINT32_C
 #define significandBits 23
 
@@ -58,9 +61,11 @@
 
 #elif defined DOUBLE_PRECISION
 
+typedef uint32_t half_rep_t;
 typedef uint64_t rep_t;
 typedef int64_t srep_t;
 typedef double fp_t;
+#define HALF_REP_C UINT32_C
 #define REP_C UINT64_C
 #define significandBits 52
 
@@ -102,9 +107,11 @@
 #elif defined QUAD_PRECISION
 #if __LDBL_MANT_DIG__ == 113 && defined(__SIZEOF_INT128__)
 #define CRT_LDBL_128BIT
+typedef uint64_t half_rep_t;
 typedef __uint128_t rep_t;
 typedef __int128_t srep_t;
 typedef long double fp_t;
+#define HALF_REP_C UINT64_C
 #define REP_C (__uint128_t)
 // Note: Since there is no explicit way to tell compiler the constant is a
 // 128-bit integer, we let the constant be casted to 128-bit integer
Index: compiler-rt/lib/builtins/fp_div_impl.inc
===
--- /dev/null
+++ compiler-rt/lib/builtins/fp_div_impl.inc
@@ -0,0 +1,389 @@
+//===-- fp_div_impl.inc - Floating point division -*- 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 implements soft-float division with the IEEE-754 default
+// rounding (to nearest, ties to even).
+//
+//===--===//
+
+#include "fp_lib.h"
+
+// The __divXf3__ function implements Newton-Raphson floating point division.
+// It uses 3 iterations for float32, 4 for float64 and 5 for float128,
+// respectively. Due to number of significant bits being roughly doubled
+// every iteration, the two modes are supported: N full-width iterations (as
+// it is done for float32 by default) and (N-1) half-width iteration plus one
+// final full-width iteration. It is expected that half-width integer
+// operations (w.r.t rep_t size

[PATCH] D85032: [builtins] Make divXf3 handle denormal results

[Anatoly Trosinenko via Phabricator via cfe-commits]

atrosinenko updated this revision to Diff 288625.
atrosinenko added a comment.

Re-upload after changing parent diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85032

Files:
  compiler-rt/lib/builtins/fp_div_impl.inc
  compiler-rt/test/builtins/Unit/divdf3_test.c
  compiler-rt/test/builtins/Unit/divsf3_test.c
  compiler-rt/test/builtins/Unit/divtf3_test.c

Index: compiler-rt/test/builtins/Unit/divtf3_test.c
===
--- compiler-rt/test/builtins/Unit/divtf3_test.c
+++ compiler-rt/test/builtins/Unit/divtf3_test.c
@@ -146,6 +146,13 @@
  UINT64_C(0xfffe)))
 return 1;
 
+// smallest normal value divided by 2.0
+if (test__divtf3(0x1.0p-16382L, 2.L, UINT64_C(0x8000), UINT64_C(0x0)))
+  return 1;
+// smallest subnormal result
+if (test__divtf3(0x1.0p-1022L, 0x1p+52L, UINT64_C(0x0), UINT64_C(0x1)))
+  return 1;
+
 // any / any
 if (test__divtf3(0x1.a23b45362464523375893ab4cdefp+5L,
  0x1.eedcbaba3a94546558237654321fp-1L,
Index: compiler-rt/test/builtins/Unit/divsf3_test.c
===
--- compiler-rt/test/builtins/Unit/divsf3_test.c
+++ compiler-rt/test/builtins/Unit/divsf3_test.c
@@ -92,5 +92,20 @@
 if (test__divsf3(0x1.0p+0F, 0x1.0001p+0F, UINT32_C(0x3f7fff00)))
   return 1;
 
+// smallest normal value divided by 2.0
+if (test__divsf3(0x1.0p-126F, 2.0F, UINT32_C(0x0040)))
+  return 1;
+// smallest subnormal result
+if (test__divsf3(0x1.0p-126F, 0x1p+23F, UINT32_C(0x0001)))
+  return 1;
+
+// some misc test cases obtained by fuzzing against h/w implementation
+if (test__divsf3(-0x1.3e75e6p-108F, -0x1.cf372p+38F, UINT32_C(0x0006)))
+  return 1;
+if (test__divsf3(0x1.e77c54p+81F, -0x1.e77c52p-47F, UINT32_C(0xff80)))
+  return 1;
+if (test__divsf3(0x1.fep-126F, 2.F, UINT32_C(0x0080)))
+  return 1;
+
 return 0;
 }
Index: compiler-rt/test/builtins/Unit/divdf3_test.c
===
--- compiler-rt/test/builtins/Unit/divdf3_test.c
+++ compiler-rt/test/builtins/Unit/divdf3_test.c
@@ -92,6 +92,13 @@
 if (test__divdf3(0x1.0p+0, 0x1.0001p+0, UINT64_C(0x3fefffe0)))
   return 1;
 
+// smallest normal value divided by 2.0
+if (test__divdf3(0x1.0p-1022, 2., UINT64_C(0x0008)))
+  return 1;
+// smallest subnormal result
+if (test__divdf3(0x1.0p-1022, 0x1.0p+52, UINT64_C(0x0001)))
+  return 1;
+
 // some misc test cases obtained by fuzzing against h/w implementation
 if (test__divdf3(0x1.fdc239dd64735p-658, -0x1.fff9364c0843fp-948, UINT64_C(0xd20fdc8fc0ceffb1)))
   return 1;
@@ -99,6 +106,12 @@
   return 1;
 if (test__divdf3(-0x1.da7dfe6048b8bp-875, 0x1.ffc7ea3ff60a4p-610, UINT64_C(0xaf5dab1fe0269e2a)))
   return 1;
+if (test__divdf3(0x1.0p-1022, 0x1.9p+5, UINT64_C(0x51eb851eb852)))
+  return 1;
+if (test__divdf3(0x1.0p-1022, 0x1.0028p+41, UINT64_C(0x07ff)))
+  return 1;
+if (test__divdf3(0x1.0p-1022, 0x1.0028p+52, UINT64_C(0x1)))
+  return 1;
 
 return 0;
 }
Index: compiler-rt/lib/builtins/fp_div_impl.inc
===
--- compiler-rt/lib/builtins/fp_div_impl.inc
+++ compiler-rt/lib/builtins/fp_div_impl.inc
@@ -328,6 +328,7 @@
   if (quotient_UQ1 < (implicitBit << 1)) {
 residualLo = (aSignificand << (significandBits + 1)) - quotient_UQ1 * bSignificand;
 writtenExponent -= 1;
+aSignificand <<= 1;
 
 // the error is doubled
   } else {
@@ -357,19 +358,25 @@
   // Now, quotient_UQ1_SB <= the correctly-rounded result
   // and may need taking NextAfter() up to 3 times (see error estimates above)
   // r = a - b * q
+  rep_t absResult;
+  if (writtenExponent > 0) {
+// Clear the implicit bit
+absResult = quotient_UQ1 & significandMask;
+// Insert the exponent
+absResult |= (rep_t)writtenExponent << significandBits;
+residualLo <<= 1;
+  } else {
+// Prevent shift amount from being negative
+if (significandBits + writtenExponent < 0)
+  return fromRep(quotientSign);
 
-  if (writtenExponent < 0) {
-// Result is definitely subnormal, flushing to zero
-return fromRep(quotientSign);
-  }
+absResult = quotient_UQ1 >> (-writtenExponent + 1);
 
-  // Clear the implicit bit
-  rep_t absResult = quotient_UQ1 & significandMask;
-  // Insert the exponent
-  absResult |= (rep_t)writtenExponent << significandBits;
+// multiplied by two to prevent shift amount to be negative
+residualLo = (aSignificand << (significandBits + writtenExponent)) - (absResult * bSignificand << 1);
+  }
 
   // Round
-  residualLo <<= 1;
   residualLo += absResult & 1; // tie

[PATCH] D85743: [CodeGen][AArch64] Support arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added a comment.

In D85743#2243188 , @c-rhodes wrote:

> In D85743#2242931 , @leonardchan 
> wrote:
>
>> Hi! The `attr-arm-sve-vector-bits-call.c` test seems to be failing on our 
>> clang builders:
>>
>> Could you take a look? Thanks.
>>
>> Builder: 
>> https://luci-milo.appspot.com/p/fuchsia/builders/ci/clang-linux-x64/b8870800848452818112?
>
> Sorry about that, I've reverted it (commit 2e7041f 
> ) whilst 
> I investigate. Thanks for raising.

The IR differences were caused by the new pass manager which is on by default 
for the Fuchsia builder. I've re-landed the patch with a fix for 
`CodeGen/attr-arm-sve-vector-bits-call.c` to use the legacy pm with 
`-fno-experimental-new-pass-manager`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85743

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

[PATCH] D81865: [clang] Use string tables for static diagnostic descriptions

[Nathan Froyd via Phabricator via cfe-commits]

froydnj updated this revision to Diff 288628.
froydnj added a comment.

Updated to use a struct instead of a union for the actual table in an effort to 
avoid UB.


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

https://reviews.llvm.org/D81865

Files:
  clang/lib/Basic/DiagnosticIDs.cpp

Index: clang/lib/Basic/DiagnosticIDs.cpp
===
--- clang/lib/Basic/DiagnosticIDs.cpp
+++ clang/lib/Basic/DiagnosticIDs.cpp
@@ -26,6 +26,78 @@
 
 namespace {
 
+struct StaticDiagInfoRec;
+
+// Store the descriptions in a separate table to avoid pointers that need to
+// be relocated, and also decrease the amount of data needed on 64-bit
+// platforms. See "How To Write Shared Libraries" by Ulrich Drepper.
+struct StaticDiagInfoDescriptionStringTable {
+#define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR, \
+ SHOWINSYSHEADER, CATEGORY)\
+  char ENUM##_desc[sizeof(DESC)];
+  // clang-format off
+#include "clang/Basic/DiagnosticCommonKinds.inc"
+#include "clang/Basic/DiagnosticDriverKinds.inc"
+#include "clang/Basic/DiagnosticFrontendKinds.inc"
+#include "clang/Basic/DiagnosticSerializationKinds.inc"
+#include "clang/Basic/DiagnosticLexKinds.inc"
+#include "clang/Basic/DiagnosticParseKinds.inc"
+#include "clang/Basic/DiagnosticASTKinds.inc"
+#include "clang/Basic/DiagnosticCommentKinds.inc"
+#include "clang/Basic/DiagnosticCrossTUKinds.inc"
+#include "clang/Basic/DiagnosticSemaKinds.inc"
+#include "clang/Basic/DiagnosticAnalysisKinds.inc"
+#include "clang/Basic/DiagnosticRefactoringKinds.inc"
+  // clang-format on
+#undef DIAG
+};
+
+const StaticDiagInfoDescriptionStringTable StaticDiagInfoDescriptions = {
+#define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR, \
+ SHOWINSYSHEADER, CATEGORY)\
+  DESC,
+  // clang-format off
+#include "clang/Basic/DiagnosticCommonKinds.inc"
+#include "clang/Basic/DiagnosticDriverKinds.inc"
+#include "clang/Basic/DiagnosticFrontendKinds.inc"
+#include "clang/Basic/DiagnosticSerializationKinds.inc"
+#include "clang/Basic/DiagnosticLexKinds.inc"
+#include "clang/Basic/DiagnosticParseKinds.inc"
+#include "clang/Basic/DiagnosticASTKinds.inc"
+#include "clang/Basic/DiagnosticCommentKinds.inc"
+#include "clang/Basic/DiagnosticCrossTUKinds.inc"
+#include "clang/Basic/DiagnosticSemaKinds.inc"
+#include "clang/Basic/DiagnosticAnalysisKinds.inc"
+#include "clang/Basic/DiagnosticRefactoringKinds.inc"
+  // clang-format on
+#undef DIAG
+};
+
+extern const StaticDiagInfoRec StaticDiagInfo[];
+
+// Stored separately from StaticDiagInfoRec to pack better.  Otherwise,
+// StaticDiagInfoRec would have extra padding on 64-bit platforms.
+const uint32_t StaticDiagInfoDescriptionOffsets[] = {
+#define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR, \
+ SHOWINSYSHEADER, CATEGORY)\
+  offsetof(StaticDiagInfoDescriptionStringTable, ENUM##_desc),
+  // clang-format off
+#include "clang/Basic/DiagnosticCommonKinds.inc"
+#include "clang/Basic/DiagnosticDriverKinds.inc"
+#include "clang/Basic/DiagnosticFrontendKinds.inc"
+#include "clang/Basic/DiagnosticSerializationKinds.inc"
+#include "clang/Basic/DiagnosticLexKinds.inc"
+#include "clang/Basic/DiagnosticParseKinds.inc"
+#include "clang/Basic/DiagnosticASTKinds.inc"
+#include "clang/Basic/DiagnosticCommentKinds.inc"
+#include "clang/Basic/DiagnosticCrossTUKinds.inc"
+#include "clang/Basic/DiagnosticSemaKinds.inc"
+#include "clang/Basic/DiagnosticAnalysisKinds.inc"
+#include "clang/Basic/DiagnosticRefactoringKinds.inc"
+  // clang-format on
+#undef DIAG
+};
+
 // Diagnostic classes.
 enum {
   CLASS_NOTE   = 0x01,
@@ -47,14 +119,16 @@
   uint16_t OptionGroupIndex;
 
   uint16_t DescriptionLen;
-  const char *DescriptionStr;
 
   unsigned getOptionGroupIndex() const {
 return OptionGroupIndex;
   }
 
   StringRef getDescription() const {
-return StringRef(DescriptionStr, DescriptionLen);
+size_t MyIndex = this - &StaticDiagInfo[0];
+uint32_t StringOffset = StaticDiagInfoDescriptionOffsets[MyIndex];
+const char* Table = reinterpret_cast(&StaticDiagInfoDescriptions);
+return StringRef(&Table[StringOffset], DescriptionLen);
   }
 
   diag::Flavor getFlavor() const {
@@ -91,16 +165,20 @@
 #undef VALIDATE_DIAG_SIZE
 #undef STRINGIFY_NAME
 
-} // namespace anonymous
-
-static const StaticDiagInfoRec StaticDiagInfo[] = {
+const StaticDiagInfoRec StaticDiagInfo[] = {
 #define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR, \
  SHOWINSYSHEADER, CATEGORY)\
   {\
-diag::ENUM, DEFAULT_SEVERITY, CLASS, DiagnosticIDs::SFINAE, NOWERROR,  \
-SHOWINSYSHEADER, CATEGORY, GROUP, STR_SIZE(DESC, uin

[PATCH] D81865: [clang] Use string tables for static diagnostic descriptions

[Nathan Froyd via Phabricator via cfe-commits]

froydnj requested review of this revision.
froydnj added a comment.

Throwing this back into the review queue.


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

https://reviews.llvm.org/D81865

___
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 marked 2 inline comments as not done.
dougpuob added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h:44
   llvm::Optional
-  GetDeclFailureInfo(const StringRef &TypeName, const NamedDecl *Decl,
+  getDeclFailureInfo(const StringRef &TypeName, const NamedDecl *Decl,
  const SourceManager &SM) const override;

aaron.ballman wrote:
> Given that the other functions in the class use the wrong style of casing, we 
> should probably leave this declaration alone so it doesn't become locally 
> inconsistent.
Do you mean create another function with three parameters for it?



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:428
 
   case IdentifierNamingCheck::CT_HungarianNotation: {
 const NamedDecl *pNamedDecl = dyn_cast(pDecl);

njames93 wrote:
> aaron.ballman wrote:
> > I feel like I must be missing something because I don't have this enum in 
> > my copy of ToT despite pulling this morning. Is the patch missing some 
> > content?
> I feel like this is an incremental diff based on the first version of this 
> patch rather than a diff from trunk. 
@aaron.ballman and @njames93, the diff is based on first version not from 
master(trunk).

Is it the best way that I should merge with the latest master(trunk) every time 
before updating the diffs ?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D86671

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



Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h:44
   llvm::Optional
-  GetDeclFailureInfo(const StringRef &TypeName, const NamedDecl *Decl,
+  getDeclFailureInfo(const StringRef &TypeName, const NamedDecl *Decl,
  const SourceManager &SM) const override;

dougpuob wrote:
> aaron.ballman wrote:
> > Given that the other functions in the class use the wrong style of casing, 
> > we should probably leave this declaration alone so it doesn't become 
> > locally inconsistent.
> Do you mean create another function with three parameters for it?
I think I got it. Keep consistent even it is wrong style of casting. 


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D86671

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

[PATCH] D82502: [PowerPC] Implement Load VSX Vector and Sign Extend and Zero Extend

[Albion Fung via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Conanap marked an inline comment as done.
Closed by commit rG331dcc43eac2: [PowerPC] Implemented Vector Load with Zero 
and Signed Extend Builtins (authored by Conanap).

Changed prior to commit:
  https://reviews.llvm.org/D82502?vs=283061&id=288637#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82502

Files:
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/builtins-ppc-p10vector.c
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/lib/Target/PowerPC/PPCISelLowering.h
  llvm/lib/Target/PowerPC/PPCInstrPrefix.td
  llvm/test/CodeGen/PowerPC/builtins-ppc-p10vsx.ll

Index: llvm/test/CodeGen/PowerPC/builtins-ppc-p10vsx.ll
===
--- llvm/test/CodeGen/PowerPC/builtins-ppc-p10vsx.ll
+++ llvm/test/CodeGen/PowerPC/builtins-ppc-p10vsx.ll
@@ -239,3 +239,181 @@
   store i64 %conv, i64* %add.ptr, align 8
   ret void
 }
+
+define dso_local <1 x i128> @vec_xl_zext(i64 %__offset, i8* nocapture readonly %__pointer) {
+; CHECK-LABEL: vec_xl_zext:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:lxvrbx v2, r4, r3
+; CHECK-NEXT:blr
+;
+; CHECK-O0-LABEL: vec_xl_zext:
+; CHECK-O0:   # %bb.0: # %entry
+; CHECK-O0-NEXT:lxvrbx vs0, r4, r3
+; CHECK-O0-NEXT:xxlor v2, vs0, vs0
+; CHECK-O0-NEXT:blr
+entry:
+  %add.ptr = getelementptr inbounds i8, i8* %__pointer, i64 %__offset
+  %0 = load i8, i8* %add.ptr, align 1
+  %conv = zext i8 %0 to i128
+  %splat.splatinsert = insertelement <1 x i128> undef, i128 %conv, i32 0
+  ret <1 x i128> %splat.splatinsert
+}
+
+define dso_local <1 x i128> @vec_xl_zext_short(i64 %__offset, i16* nocapture readonly %__pointer) {
+; CHECK-LABEL: vec_xl_zext_short:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:sldi r3, r3, 1
+; CHECK-NEXT:lxvrhx v2, r4, r3
+; CHECK-NEXT:blr
+;
+; CHECK-O0-LABEL: vec_xl_zext_short:
+; CHECK-O0:   # %bb.0: # %entry
+; CHECK-O0-NEXT:sldi r3, r3, 1
+; CHECK-O0-NEXT:lxvrhx vs0, r4, r3
+; CHECK-O0-NEXT:xxlor v2, vs0, vs0
+; CHECK-O0-NEXT:blr
+entry:
+  %add.ptr = getelementptr inbounds i16, i16* %__pointer, i64 %__offset
+  %0 = load i16, i16* %add.ptr, align 2
+  %conv = zext i16 %0 to i128
+  %splat.splatinsert = insertelement <1 x i128> undef, i128 %conv, i32 0
+  ret <1 x i128> %splat.splatinsert
+}
+
+define dso_local <1 x i128> @vec_xl_zext_word(i64 %__offset, i32* nocapture readonly %__pointer) {
+; CHECK-LABEL: vec_xl_zext_word:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:sldi r3, r3, 2
+; CHECK-NEXT:lxvrwx v2, r4, r3
+; CHECK-NEXT:blr
+;
+; CHECK-O0-LABEL: vec_xl_zext_word:
+; CHECK-O0:   # %bb.0: # %entry
+; CHECK-O0-NEXT:sldi r3, r3, 2
+; CHECK-O0-NEXT:lxvrwx vs0, r4, r3
+; CHECK-O0-NEXT:xxlor v2, vs0, vs0
+; CHECK-O0-NEXT:blr
+entry:
+  %add.ptr = getelementptr inbounds i32, i32* %__pointer, i64 %__offset
+  %0 = load i32, i32* %add.ptr, align 4
+  %conv = zext i32 %0 to i128
+  %splat.splatinsert = insertelement <1 x i128> undef, i128 %conv, i32 0
+  ret <1 x i128> %splat.splatinsert
+}
+
+define dso_local <1 x i128> @vec_xl_zext_dw(i64 %__offset, i64* nocapture readonly %__pointer) {
+; CHECK-LABEL: vec_xl_zext_dw:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:sldi r3, r3, 3
+; CHECK-NEXT:lxvrdx v2, r4, r3
+; CHECK-NEXT:blr
+;
+; CHECK-O0-LABEL: vec_xl_zext_dw:
+; CHECK-O0:   # %bb.0: # %entry
+; CHECK-O0-NEXT:sldi r3, r3, 3
+; CHECK-O0-NEXT:lxvrdx vs0, r4, r3
+; CHECK-O0-NEXT:xxlor v2, vs0, vs0
+; CHECK-O0-NEXT:blr
+entry:
+  %add.ptr = getelementptr inbounds i64, i64* %__pointer, i64 %__offset
+  %0 = load i64, i64* %add.ptr, align 8
+  %conv = zext i64 %0 to i128
+  %splat.splatinsert = insertelement <1 x i128> undef, i128 %conv, i32 0
+  ret <1 x i128> %splat.splatinsert
+}
+
+define dso_local <1 x i128> @vec_xl_sext_b(i64 %offset, i8* %p) {
+; CHECK-LABEL: vec_xl_sext_b:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:lbzx r3, r4, r3
+; CHECK-NEXT:extsb r3, r3
+; CHECK-NEXT:sradi r4, r3, 63
+; CHECK-NEXT:mtvsrdd v2, r4, r3
+; CHECK-NEXT:blr
+;
+; CHECK-O0-LABEL: vec_xl_sext_b:
+; CHECK-O0:   # %bb.0: # %entry
+; CHECK-O0-NEXT:lbzx r3, r4, r3
+; CHECK-O0-NEXT:extsb r3, r3
+; CHECK-O0-NEXT:sradi r4, r3, 63
+; CHECK-O0-NEXT:mtvsrdd v2, r4, r3
+; CHECK-O0-NEXT:blr
+entry:
+  %add.ptr = getelementptr inbounds i8, i8* %p, i64 %offset
+  %0 = load i8, i8* %add.ptr, align 1
+  %conv = sext i8 %0 to i128
+  %splat.splatinsert = insertelement <1 x i128> undef, i128 %conv, i32 0
+  ret <1 x i128> %splat.splatinsert
+}
+
+define dso_local <1 x i128> @vec_xl_sext_h(i64 %offset, i16* %p) {
+; CHECK-LABEL: vec_xl_sext_h:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:sldi r3, r3, 1
+; CHECK-NEXT:lhax r3, r4, r3
+; CHECK-NE

[clang] 331dcc4 - [PowerPC] Implemented Vector Load with Zero and Signed Extend Builtins

[Albion Fung via cfe-commits]

Author: Albion Fung
Date: 2020-08-28T11:28:58-05:00
New Revision: 331dcc43eac28b8e659f928fd1f1ce7fd091e1e3

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

LOG: [PowerPC] Implemented Vector Load with Zero and Signed Extend Builtins

This patch implements the builtins for Vector Load with Zero and Signed Extend 
Builtins (lxvr_x for b, h, w, d), and adds the appropriate test cases for these 
builtins. The builtins utilize the vector load instructions itnroduced with ISA 
3.1.

Differential Revision:  https://reviews.llvm.org/D82502#inline-797941

Added: 


Modified: 
clang/lib/Headers/altivec.h
clang/test/CodeGen/builtins-ppc-p10vector.c
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
llvm/lib/Target/PowerPC/PPCISelLowering.h
llvm/lib/Target/PowerPC/PPCInstrPrefix.td
llvm/test/CodeGen/PowerPC/builtins-ppc-p10vsx.ll

Removed: 




diff  --git a/clang/lib/Headers/altivec.h b/clang/lib/Headers/altivec.h
index 6583b0f22a16..5bc0e23792d4 100644
--- a/clang/lib/Headers/altivec.h
+++ b/clang/lib/Headers/altivec.h
@@ -16557,6 +16557,54 @@ vec_xl_be(signed long long  __offset, unsigned 
__int128 *__ptr) {
   #define vec_xl_be vec_xl
 #endif
 
+#if defined(__POWER10_VECTOR__) && defined(__VSX__)
+
+/* vect_xl_sext */
+
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_xl_sext(signed long long __offset, signed char *__pointer) {
+  return (vector unsigned __int128)*(__pointer + __offset);
+}
+
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_xl_sext(signed long long __offset, signed short *__pointer) {
+  return (vector unsigned __int128)*(__pointer + __offset);
+}
+
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_xl_sext(signed long long __offset, signed int *__pointer) {
+  return (vector unsigned __int128)*(__pointer + __offset);
+}
+
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_xl_sext(signed long long __offset, signed long long *__pointer) {
+  return (vector unsigned __int128)*(__pointer + __offset);
+}
+
+/* vec_xl_zext */
+
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_xl_zext(signed long long __offset, unsigned char *__pointer) {
+  return (vector unsigned __int128)*(__pointer + __offset);
+}
+
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_xl_zext(signed long long __offset, unsigned short *__pointer) {
+  return (vector unsigned __int128)*(__pointer + __offset);
+}
+
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_xl_zext(signed long long __offset, unsigned int *__pointer) {
+  return (vector unsigned __int128)*(__pointer + __offset);
+}
+
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_xl_zext(signed long long __offset, unsigned long long *__pointer) {
+  return (vector unsigned __int128)*(__pointer + __offset);
+}
+
+#endif
+
 /* vec_xst */
 
 static inline __ATTRS_o_ai void vec_xst(vector signed char __vec,

diff  --git a/clang/test/CodeGen/builtins-ppc-p10vector.c 
b/clang/test/CodeGen/builtins-ppc-p10vector.c
index 16e468b62318..09477891bb06 100644
--- a/clang/test/CodeGen/builtins-ppc-p10vector.c
+++ b/clang/test/CodeGen/builtins-ppc-p10vector.c
@@ -20,10 +20,14 @@ vector unsigned long long vulla, vullb, vullc;
 vector unsigned __int128 vui128a, vui128b, vui128c;
 vector float vfa, vfb;
 vector double vda, vdb;
-unsigned int uia, uib;
-unsigned char uca;
-unsigned short usa;
-unsigned long long ulla;
+signed int *iap;
+unsigned int uia, uib, *uiap;
+signed char *cap;
+unsigned char uca, *ucap;
+signed short *sap;
+unsigned short usa, *usap;
+signed long long *llap, llb;
+unsigned long long ulla, *ullap;
 
 vector signed long long test_vec_mul_sll(void) {
   // CHECK: mul <2 x i64>
@@ -911,3 +915,59 @@ int test_vec_test_lsbb_all_zeros(void) {
   // CHECK-NEXT: ret i32
   return vec_test_lsbb_all_zeros(vuca);
 }
+
+vector signed __int128 test_vec_xl_sext_i8(void) {
+  // CHECK: load i8
+  // CHECK: sext i8
+  // CHECK: ret <1 x i128>
+  return vec_xl_sext(llb, cap);
+}
+
+vector signed __int128 test_vec_xl_sext_i16(void) {
+  // CHECK: load i16
+  // CHECK: sext i16
+  // CHECK: ret <1 x i128>
+  return vec_xl_sext(llb, sap);
+}
+
+vector signed __int128 test_vec_xl_sext_i32(void) {
+  // CHECK: load i32
+  // CHECK: sext i32
+  // CHECK: ret <1 x i128>
+  return vec_xl_sext(llb, iap);
+}
+
+vector signed __int128 test_vec_xl_sext_i64(void) {
+  // CHECK: load i64
+  // CHECK: sext i64
+  // CHECK: ret <1 x i128>
+  return vec_xl_sext(llb, llap);
+}
+
+vector unsigned __int128 test_vec_xl_zext_i8(void) {
+  // CHECK: load i8
+  // CHECK: zext i8
+  // CHECK: ret <1 x i128>
+  return vec_xl_zext(llb, ucap);
+}
+
+vector unsigned __int128 test_vec_xl_zext_i16(void) {
+  // CHECK: load i16
+  // CHECK: zext i16
+  // CHECK: ret <1 x i128>
+  return 

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h:44
   llvm::Optional
-  GetDeclFailureInfo(const StringRef &TypeName, const NamedDecl *Decl,
+  getDeclFailureInfo(const StringRef &TypeName, const NamedDecl *Decl,
  const SourceManager &SM) const override;

dougpuob wrote:
> dougpuob wrote:
> > aaron.ballman wrote:
> > > Given that the other functions in the class use the wrong style of 
> > > casing, we should probably leave this declaration alone so it doesn't 
> > > become locally inconsistent.
> > Do you mean create another function with three parameters for it?
> I think I got it. Keep consistent even it is wrong style of casting. 
Yes, exactly!



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:428
 
   case IdentifierNamingCheck::CT_HungarianNotation: {
 const NamedDecl *pNamedDecl = dyn_cast(pDecl);

dougpuob wrote:
> njames93 wrote:
> > aaron.ballman wrote:
> > > I feel like I must be missing something because I don't have this enum in 
> > > my copy of ToT despite pulling this morning. Is the patch missing some 
> > > content?
> > I feel like this is an incremental diff based on the first version of this 
> > patch rather than a diff from trunk. 
> @aaron.ballman and @njames93, the diff is based on first version not from 
> master(trunk).
> 
> Is it the best way that I should merge with the latest master(trunk) every 
> time before updating the diffs ?
Patches are typically a diff against trunk unless you have a series of related 
patches that build on top of one another (in which case, Phab has a way to mark 
related reviews so reviewers can still track the full context of the patch 
set). So when you create the diff, it should typically be against trunk.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D86671

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

[PATCH] D86795: [PowerPC] Implement builtins for xvcvspbf16 and xvcvbf16spn

[Amy Kwan via Phabricator via cfe-commits]

amyk created this revision.
amyk added reviewers: bsaleil, lei, nemanjai, power-llvm-team, PowerPC.
amyk added projects: LLVM, PowerPC.
Herald added subscribers: llvm-commits, cfe-commits, shchenz, hiraditya.
Herald added a project: clang.
amyk requested review of this revision.

This patch adds the builtin implementation for the xvcvspbf16 and xvcvbf16spn 
instructions.

Depends on D86794 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86795

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/test/CodeGen/builtins-ppc-p10vector.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCInstrPrefix.td
  llvm/test/CodeGen/PowerPC/bfloat16-outer-product.ll

Index: llvm/test/CodeGen/PowerPC/bfloat16-outer-product.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/bfloat16-outer-product.ll
@@ -0,0 +1,52 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:   -mcpu=pwr10 -ppc-asm-full-reg-names \
+; RUN:   -ppc-vsr-nums-as-vr < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
+; RUN:   -mcpu=pwr10 -ppc-asm-full-reg-names \
+; RUN:   -ppc-vsr-nums-as-vr < %s | FileCheck %s --check-prefix=CHECK-BE
+
+; Function Attrs: nofree nounwind writeonly
+define dso_local void @test60(i8* nocapture readnone %vqp, i8* nocapture readnone %vpp, <16 x i8> %vc, i8* nocapture %resp) {
+; CHECK-LABEL: test60:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:xvcvspbf16 vs0, v2
+; CHECK-NEXT:stxv vs0, 0(r7)
+; CHECK-NEXT:blr
+;
+; CHECK-BE-LABEL: test60:
+; CHECK-BE:   # %bb.0: # %entry
+; CHECK-BE-NEXT:xvcvspbf16 vs0, v2
+; CHECK-BE-NEXT:stxv vs0, 0(r7)
+; CHECK-BE-NEXT:blr
+entry:
+  %0 = tail call <16 x i8> @llvm.ppc.vsx.xvcvspbf16(<16 x i8> %vc)
+  %1 = bitcast i8* %resp to <16 x i8>*
+  store <16 x i8> %0, <16 x i8>* %1, align 16
+  ret void
+}
+; Function Attrs: nounwind readnone
+declare <16 x i8> @llvm.ppc.vsx.xvcvspbf16(<16 x i8>)
+
+; Function Attrs: nofree nounwind writeonly
+define dso_local void @test61(i8* nocapture readnone %vqp, i8* nocapture readnone %vpp, <16 x i8> %vc, i8* nocapture %resp) {
+; CHECK-LABEL: test61:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:xvcvbf16spn vs0, v2
+; CHECK-NEXT:stxv vs0, 0(r7)
+; CHECK-NEXT:blr
+;
+; CHECK-BE-LABEL: test61:
+; CHECK-BE:   # %bb.0: # %entry
+; CHECK-BE-NEXT:xvcvbf16spn vs0, v2
+; CHECK-BE-NEXT:stxv vs0, 0(r7)
+; CHECK-BE-NEXT:blr
+entry:
+  %0 = tail call <16 x i8> @llvm.ppc.vsx.xvcvbf16spn(<16 x i8> %vc)
+  %1 = bitcast i8* %resp to <16 x i8>*
+  store <16 x i8> %0, <16 x i8>* %1, align 16
+  ret void
+}
+
+; Function Attrs: nounwind readnone
+declare <16 x i8> @llvm.ppc.vsx.xvcvbf16spn(<16 x i8>)
Index: llvm/lib/Target/PowerPC/PPCInstrPrefix.td
===
--- llvm/lib/Target/PowerPC/PPCInstrPrefix.td
+++ llvm/lib/Target/PowerPC/PPCInstrPrefix.td
@@ -506,6 +506,11 @@
 def PairedVectorMemops : Predicate<"PPCSubTarget->pairedVectorMemops()">;
 def MMA : Predicate<"PPCSubTarget->hasMMA()">;
 
+def RCCp {
+  dag AToVSRC = (COPY_TO_REGCLASS $XA, VSRC);
+  dag BToVSRC = (COPY_TO_REGCLASS $XB, VSRC);
+}
+
 let Predicates = [PrefixInstrs] in {
   let Interpretation64Bit = 1, isCodeGenOnly = 1 in {
 defm PADDI8 :
@@ -1333,6 +1338,13 @@
 (v1i128 (VSRAQ v1i128:$VRA, v1i128:$VRB))>;
 }
 
+let Predicates = [IsISA3_1, HasVSX] in {
+  def : Pat<(v16i8 (int_ppc_vsx_xvcvspbf16 v16i8:$XA)),
+(COPY_TO_REGCLASS (XVCVSPBF16 RCCp.AToVSRC), VRRC)>;
+  def : Pat<(v16i8 (int_ppc_vsx_xvcvbf16spn v16i8:$XA)),
+(COPY_TO_REGCLASS (XVCVBF16SPN RCCp.AToVSRC), VRRC)>;
+}
+
 let AddedComplexity = 400, Predicates = [IsISA3_1] in {
   def : Pat<(truncstorei8 (i32 (vector_extract v16i8:$rS, 0)), xoaddr:$src),
 (STXVRBX (COPY_TO_REGCLASS $rS, VSRC), xoaddr:$src)>;
Index: llvm/include/llvm/IR/IntrinsicsPowerPC.td
===
--- llvm/include/llvm/IR/IntrinsicsPowerPC.td
+++ llvm/include/llvm/IR/IntrinsicsPowerPC.td
@@ -1124,6 +1124,12 @@
 def int_ppc_vsx_xvcvhpsp :
   PowerPC_VSX_Intrinsic<"xvcvhpsp", [llvm_v4f32_ty],
 [llvm_v8i16_ty],[IntrNoMem]>;
+def int_ppc_vsx_xvcvspbf16 :
+  PowerPC_VSX_Intrinsic<"xvcvspbf16", [llvm_v16i8_ty],
+[llvm_v16i8_ty], [IntrNoMem]>;
+def int_ppc_vsx_xvcvbf16spn :
+  PowerPC_VSX_Intrinsic<"xvcvbf16spn", [llvm_v16i8_ty],
+[llvm_v16i8_ty], [IntrNoMem]>;
 def int_ppc_vsx_xxextractuw :
   PowerPC_VSX_Intrinsic<"xxextractuw",[llvm_v2i64_ty],
 [llvm_v2i64_ty,llvm_i32_ty], [IntrNoMem]>;
Index: clang/test/CodeGen/builtins-ppc-p10vector.c
=

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

[Chris Hamilton via Phabricator via cfe-commits]

chrish_ericsson_atx created this revision.
Herald added subscribers: cfe-commits, arphaman.
Herald added a project: clang.
chrish_ericsson_atx requested review of this revision.

Check applied to unbounded (incomplete) arrays and pointers
to spot cases where the computed address is beyond the
largest possible addressable extent of the array, based
on the address space in which the array is delcared, or
which the pointer refers to.

Check helps to avoid cases of nonsense pointer math and
array indexing which could lead to linker failures or
runtime exceptions.  Of particular interest when building
for embedded systems with small address spaces.

Change-Id: I836042912f0f7ba4ee774ac03d2fb47d987709e2


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86796

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/const-eval.c
  clang/test/Sema/unbounded-array-bounds.c
  clang/test/SemaCXX/constant-expression-cxx1y.cpp

Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -1018,8 +1018,9 @@
 }
 
 constexpr void PR28739(int n) { // expected-error {{never produces a constant}}
-  int *p = &n;
+  int *p = &n;  // expected-note {{declared here}}
   p += (__int128)(unsigned long)-1; // expected-note {{cannot refer to element 18446744073709551615 of non-array object in a constant expression}}
+  // expected-warning@-1 {{refers past the last possible element}}
 }
 
 constexpr void Void(int n) {
Index: clang/test/Sema/unbounded-array-bounds.c
===
--- /dev/null
+++ clang/test/Sema/unbounded-array-bounds.c
@@ -0,0 +1,58 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-X86-ADDR64 %s  \
+// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple i386-pc-linux-gnu   -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-I386-ADDR32 %s \
+// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple avr-pc-linux-gnu-fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-AVR-ADDR16 %s  \
+// RUN:  --implicit-check-not 'past the last possible element'
+
+struct S {
+  long long a;
+  char b;
+  long long c;
+  short d;
+};
+
+struct S s[];
+
+void f1() {
+  ++s[3].a;
+  ++s[7073650413200313099].b;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  ++s[7073650].c;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+long long ll[];
+
+void f2() {
+  ++ll[3];
+  ++ll[2705843009213693952];
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 2305843009213693952 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+  ++ll[847073650];
+  // CHECK-I386-ADDR32: :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+}
+
+void f3(struct S p[]) {
+  ++p[3].a;
+  ++p[7073650413200313099].b;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  ++p[7073650].c;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+void f4(struct S *p) {
+  p += 3;
+  p += 7073650413200313099;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:3: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:3: warning: {{.*}} past the la

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

[Chris Hamilton via Phabricator via cfe-commits]

chrish_ericsson_atx updated this revision to Diff 288649.
chrish_ericsson_atx added a comment.

Removed Change-Id from commit log message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86796

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/const-eval.c
  clang/test/Sema/unbounded-array-bounds.c
  clang/test/SemaCXX/constant-expression-cxx1y.cpp

Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -1018,8 +1018,9 @@
 }
 
 constexpr void PR28739(int n) { // expected-error {{never produces a constant}}
-  int *p = &n;
+  int *p = &n;  // expected-note {{declared here}}
   p += (__int128)(unsigned long)-1; // expected-note {{cannot refer to element 18446744073709551615 of non-array object in a constant expression}}
+  // expected-warning@-1 {{refers past the last possible element}}
 }
 
 constexpr void Void(int n) {
Index: clang/test/Sema/unbounded-array-bounds.c
===
--- /dev/null
+++ clang/test/Sema/unbounded-array-bounds.c
@@ -0,0 +1,58 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-X86-ADDR64 %s  \
+// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple i386-pc-linux-gnu   -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-I386-ADDR32 %s \
+// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple avr-pc-linux-gnu-fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-AVR-ADDR16 %s  \
+// RUN:  --implicit-check-not 'past the last possible element'
+
+struct S {
+  long long a;
+  char b;
+  long long c;
+  short d;
+};
+
+struct S s[];
+
+void f1() {
+  ++s[3].a;
+  ++s[7073650413200313099].b;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  ++s[7073650].c;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+long long ll[];
+
+void f2() {
+  ++ll[3];
+  ++ll[2705843009213693952];
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 2305843009213693952 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+  ++ll[847073650];
+  // CHECK-I386-ADDR32: :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+}
+
+void f3(struct S p[]) {
+  ++p[3].a;
+  ++p[7073650413200313099].b;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  ++p[7073650].c;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+void f4(struct S *p) {
+  p += 3;
+  p += 7073650413200313099;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:3: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:3: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:3: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  p += 7073650;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:3: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
Index: clang/test/Sema/const-eval.c
===
--- clang/test/Sem

[PATCH] D85743: [CodeGen][AArch64] Support arm_sve_vector_bits attribute

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

> The IR differences were caused by the new pass manager which is on by default 
> for the Fuchsia builder. I've re-landed the patch with a fix for 
> `CodeGen/attr-arm-sve-vector-bits-call.c` to use the legacy pm with 
> `-fno-experimental-new-pass-manager`.

Thanks for the update! We do have the new PM on by default, but I'm surprised 
that this wouldn't appear on `clang-x86_64-debian-new-pass-manager-fast` which 
also tests the new PM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85743

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

[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added inline comments.



Comment at: clang/lib/Sema/SemaCast.cpp:895
+  if (!Self.getLangOpts().RTTIData) {
+bool isMSVC = Self.getDiagnostics().getDiagnosticOptions().getFormat() ==
+  DiagnosticOptions::MSVC;

hans wrote:
> zequanwu wrote:
> > hans wrote:
> > > I'm not sure isMSVC is the best name (it's not clear what aspect is MSVC 
> > > exactly -- in this case it's the diagnostics format).
> > > 
> > > It's possible to target MSVC both with clang-cl and with regular clang.
> > > 
> > > For example, one could use
> > > 
> > >   clang-cl /c /tmp/a.cpp
> > > 
> > > or
> > > 
> > >   clang -c /tmp/a.cpp -target i686-pc-windows-msvc19.11.0 -fms-extensions
> > > 
> > > 
> > > My understanding is that the purpose of "isMSVC" here is to try and 
> > > detect if we're using clang-cl or clang so that the diagnostic can say 
> > > "/GR-" or "-fno-rtti-data". So maybe it's better to call it "isClangCL" 
> > > or something like that.
> > > 
> > > Also, I don't think we should check "isMSVC" in the if-statement below. 
> > > We want the warning to fire both when using clang and clang-cl: as long 
> > > as -fno-rtti-data or /GR- is used, the warning makes sense.
> > > 
> > > So I think the code could be more like:
> > > 
> > > ```
> > > if (!Self.getLangOpts().RTTIData && !DestPointee->isVoidType()) {
> > >   bool isClangCL = ...;
> > >   Self.Diag(...) << isClangCL;
> > > }
> > > ```
> > MSVC will warn even if the DestPointee is void type. What I thought is if 
> > invoked by clang-cl warn regardless of DeskPointee type. If invoked by 
> > clang, warn if it's not void type. 
> > https://godbolt.org/z/475q5v. I noticed MSVC won't warn at typeid if /GR- 
> > is given. Probably I should remove the warning in typeid.
> If it's true the casting to void* doesn't need RTTI data (I think it is, but 
> would be good to verify), then it doesn't make sense to warn. We don't have 
> to follow MSVC's behavior when it doesn't make sense :)
> 
> Similar reasoning for typeid() - I assume it won't work with /GR- also with 
> MSVC, so warning about it probably makes sense.
In clang, I believe that dynamic_cast to void* doesn't use RTTI data: 
https://godbolt.org/z/Kbr7Mq
Looks like MSVC only warns if the operand of typeid is not pointer: 
https://godbolt.org/z/chcMcn



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86369

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

[PATCH] D73425: [PPC] Fix platform definitions when compiling FreeBSD powerpc64 as LE

[Ed Maste via Phabricator via cfe-commits]

emaste added inline comments.



Comment at: clang/lib/Basic/Targets.cpp:361-362
   return new LinuxTargetInfo(Triple, Opts);
+case llvm::Triple::FreeBSD:
+  return new FreeBSDTargetInfo(Triple, Opts);
 case llvm::Triple::NetBSD:

Bdragon28 wrote:
> Bdragon28 wrote:
> > emaste wrote:
> > > List was previously in alpha order
> > Yes. However, I am following the ordering of the ppc64 and powerpc triples.
> that is, the ordering is "Linux, the BSDs, Embedded targets, Commercial 
> targets" for ppc*.
Ok, as long as it's consistent I'm happy with it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73425

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

[PATCH] D86802: [Modules] Don't parse/load explicit module maps if modules are disabled

[Andrew Gallagher via Phabricator via cfe-commits]

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

In some build environments/systems, flags for explicit module map files
may be propagated up to dependents which may not choose to enable use of
modules (e.g. if a particular compilation doesn't currently work in
modular builds).

In this case, the explicit module files are still passed, even though
they're not used (and can trigger additional I/O at header search path,
like realpath'ing include roots if an explicit  module map contains a
umbrella dir).

This diff avoids parsing/loading these module map files if modules
haven't been enabled.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86802

Files:
  clang/lib/Frontend/FrontendAction.cpp


Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -807,12 +807,14 @@
 goto failure;
 
   // If we were asked to load any module map files, do so now.
-  for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
-if (auto File = CI.getFileManager().getFile(Filename))
-  CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
-  *File, /*IsSystem*/false);
-else
-  CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+  if (CI.getLangOpts().Modules) {
+for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
+  if (auto File = CI.getFileManager().getFile(Filename))
+CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
+*File, /*IsSystem*/ false);
+  else
+CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+}
   }
 
   // Add a module declaration scope so that modules from -fmodule-map-file


Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -807,12 +807,14 @@
 goto failure;
 
   // If we were asked to load any module map files, do so now.
-  for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
-if (auto File = CI.getFileManager().getFile(Filename))
-  CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
-  *File, /*IsSystem*/false);
-else
-  CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+  if (CI.getLangOpts().Modules) {
+for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
+  if (auto File = CI.getFileManager().getFile(Filename))
+CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
+*File, /*IsSystem*/ false);
+  else
+CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+}
   }
 
   // Add a module declaration scope so that modules from -fmodule-map-file
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 8782c72 - Strength-reduce SmallVectors to arrays. NFCI.

[Benjamin Kramer via cfe-commits]

Author: Benjamin Kramer
Date: 2020-08-28T21:14:20+02:00
New Revision: 8782c727655942c9aa4c80d698c9ba575510799c

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

LOG: Strength-reduce SmallVectors to arrays. NFCI.

Added: 


Modified: 
clang/lib/Driver/ToolChains/AMDGPU.cpp
llvm/lib/IR/DebugInfoMetadata.cpp
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
mlir/lib/Dialect/Linalg/Transforms/Fusion.cpp
mlir/lib/Dialect/SPIRV/SPIRVOps.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp 
b/clang/lib/Driver/ToolChains/AMDGPU.cpp
index b11ce41de479..71acf3ed3281 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -245,9 +245,9 @@ void RocmInstallationDetector::detectDeviceLibrary() {
 // - ${ROCM_ROOT}/lib/*
 // - ${ROCM_ROOT}/lib/bitcode/*
 // so try to detect these layouts.
-static llvm::SmallVector SubDirsList[] = {
+static constexpr std::array SubDirsList[] = {
 {"amdgcn", "bitcode"},
-{"lib"},
+{"lib", ""},
 {"lib", "bitcode"},
 };
 

diff  --git a/llvm/lib/IR/DebugInfoMetadata.cpp 
b/llvm/lib/IR/DebugInfoMetadata.cpp
index f20270810ed6..129a52b710c6 100644
--- a/llvm/lib/IR/DebugInfoMetadata.cpp
+++ b/llvm/lib/IR/DebugInfoMetadata.cpp
@@ -133,7 +133,7 @@ const DILocation *DILocation::getMergedLocation(const 
DILocation *LocA,
 }
 
 Optional DILocation::encodeDiscriminator(unsigned BD, unsigned DF, 
unsigned CI) {
-  SmallVector Components = {BD, DF, CI};
+  std::array Components = {BD, DF, CI};
   uint64_t RemainingWork = 0U;
   // We use RemainingWork to figure out if we have no remaining components to
   // encode. For example: if BD != 0 but DF == 0 && CI == 0, we don't need to

diff  --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp 
b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index e68183dc46a2..e1b79393f25f 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -15710,8 +15710,8 @@ SDValue 
AArch64TargetLowering::LowerFixedLengthVectorSetccToSVE(
   auto Pg = getPredicateForFixedLengthVector(DAG, DL, InVT);
 
   EVT CmpVT = Pg.getValueType();
-  SmallVector CmpOps = {Pg, Op1, Op2, Op.getOperand(2)};
-  auto Cmp = DAG.getNode(AArch64ISD::SETCC_MERGE_ZERO, DL, CmpVT, CmpOps);
+  auto Cmp = DAG.getNode(AArch64ISD::SETCC_MERGE_ZERO, DL, CmpVT,
+ {Pg, Op1, Op2, Op.getOperand(2)});
 
   EVT PromoteVT = ContainerVT.changeTypeToInteger();
   auto Promote = DAG.getBoolExtOrTrunc(Cmp, DL, PromoteVT, InVT);

diff  --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp 
b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 75cb87345949..ae2471969160 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -3156,7 +3156,7 @@ static bool mergeConditionalStoreToAddress(BasicBlock 
*PTB, BasicBlock *PFB,
 return true;
   };
 
-  const SmallVector FreeStores = {PStore, QStore};
+  const std::array FreeStores = {PStore, QStore};
   if (!MergeCondStoresAggressively &&
   (!IsWorthwhile(PTB, FreeStores) || !IsWorthwhile(PFB, FreeStores) ||
!IsWorthwhile(QTB, FreeStores) || !IsWorthwhile(QFB, FreeStores)))

diff  --git a/mlir/lib/Dialect/Linalg/Transforms/Fusion.cpp 
b/mlir/lib/Dialect/Linalg/Transforms/Fusion.cpp
index 126228bce25a..6c0c841451da 100644
--- a/mlir/lib/Dialect/Linalg/Transforms/Fusion.cpp
+++ b/mlir/lib/Dialect/Linalg/Transforms/Fusion.cpp
@@ -377,11 +377,10 @@ fuseProducerOfDep(OpBuilder &b, LinalgOp consumer, 
unsigned consumerIdx,
 Optional mlir::linalg::fuseProducerOf(
 OpBuilder &b, LinalgOp consumer, unsigned consumerIdx,
 const LinalgDependenceGraph &graph, OperationFolder *folder) {
-  SmallVector deps = {
-  LinalgDependenceGraph::DependenceType::RAW,
-  LinalgDependenceGraph::DependenceType::WAW,
-  };
-  for (auto dep : deps) {
+  for (auto dep : {
+   LinalgDependenceGraph::DependenceType::RAW,
+   LinalgDependenceGraph::DependenceType::WAW,
+   }) {
 if (auto res =
 fuseProducerOfDep(b, consumer, consumerIdx, graph, folder, dep))
   return res;

diff  --git a/mlir/lib/Dialect/SPIRV/SPIRVOps.cpp 
b/mlir/lib/Dialect/SPIRV/SPIRVOps.cpp
index 7aeecdbbac35..83de4cc2e8fe 100644
--- a/mlir/lib/Dialect/SPIRV/SPIRVOps.cpp
+++ b/mlir/lib/Dialect/SPIRV/SPIRVOps.cpp
@@ -2849,9 +2849,9 @@ static ParseResult 
parseCooperativeMatrixLoadNVOp(OpAsmParser &parser,
   parser.parseType(ptrType) || parser.parseKeywordType("as", elementType)) 
{
 return failure();
   }
-  SmallVector OperandType = {ptrType, strideType, columnMajorType};
-  if (parser.resolveOperands(operandInfo, OperandType,

[PATCH] D86802: [Modules] Don't parse/load explicit module maps if modules are disabled

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added a comment.

Hi Andrew, thanks for improving this. I think this makes sense: dependents can 
choose to not use modules without having to trigger the build system to rebuild 
all dependencies. Can you add a simple testcase to prove the point of the 
change?




Comment at: clang/lib/Frontend/FrontendAction.cpp:814
+CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
+*File, /*IsSystem*/ false);
+  else

Is this clang-formatted? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86802

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

[PATCH] D86802: [Modules] Don't parse/load explicit module maps if modules are disabled

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg added a comment.

> Can you add a simple testcase to prove the point of the change?

Yup, will do!




Comment at: clang/lib/Frontend/FrontendAction.cpp:814
+CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
+*File, /*IsSystem*/ false);
+  else

bruno wrote:
> Is this clang-formatted? 
Ahh, yeah.  Should I avoid this sort of thing (and e.g. instead do this in 
explicit codemod/refactoring diffs)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86802

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

[PATCH] D86802: [Modules] Don't parse/load explicit module maps if modules are disabled

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.
Herald added a subscriber: danielkiss.

How about a malformed module map is not loaded and gives no errors?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86802

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

[PATCH] D86802: [Modules] Don't parse/load explicit module maps if modules are disabled

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg added a comment.

> How about a malformed module map is not loaded and gives no errors?

Heh yeah, was thinking the same :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86802

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

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.
Herald added a subscriber: danielkiss.

New compile-time numbers: 
https://llvm-compile-time-tracker.com/compare.php?from=d7c119d89c5f6d0789cfd0a139c80e23912c0bb0&to=e0a1a6cac1b982023f8ceba8285d1ee7bc96bd32&stat=instructions
 The regression is now reduced to 0.2%. I assume that's the overhead of those 
CallbackVHs.

I have no familiarity with BFI, so possibly stupid question: There is already 
some similar handling as part of BFIImpl here: 
https://github.com/llvm/llvm-project/blob/0f14b2e6cbb54c84ed3b00b0db521f5ce2d1e3f2/llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h#L1043-L1058
 What is the difference to that / why are both needed?




Comment at: llvm/include/llvm/Analysis/BlockFrequencyInfo.h:43
+  class BFICallbackVH final : public CallbackVH {
+std::weak_ptr BFI;
+const BasicBlock *BB;

Is the shared_ptr/weak_ptr usage here necessary? This type of map deletion 
CallbackVH is a common pattern, but this is the first time I see it with 
weak_ptr.



Comment at: llvm/include/llvm/Analysis/BlockFrequencyInfo.h:44
+std::weak_ptr BFI;
+const BasicBlock *BB;
+

It should not be necessary to explicitly store `BB` here, it is already part of 
the value handle (you can access it via `*this` for example).



Comment at: llvm/include/llvm/Analysis/BlockFrequencyInfo.h:48
+BFICallbackVH(std::shared_ptr BFI, const BasicBlock *BB);
+virtual ~BFICallbackVH() = default;
+

I don't believe this virtual dtor is needed (class is final). For that matter, 
the method below also don't need to marked virtual.



Comment at: llvm/lib/Analysis/BlockFrequencyInfo.cpp:212
+  for (const auto *BB : RegisteredBlocks)
+BlockWatchers.emplace_back(BFICallbackVH(BFI, BB));
+

May want to clear BlockWatchers in `BlockFrequencyInfo::releaseMemory()`?



Comment at: llvm/lib/Transforms/Scalar/LoopDistribute.cpp:1062
+LoopStandardAnalysisResults AR = {AA,  AC,  DT,  LI, SE,
+  TLI, TTI, nullptr, nullptr};
 return LAM.getResult(L, AR);

Huh, surprised that clang-format allows this.


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

https://reviews.llvm.org/D86156

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

[PATCH] D85676: [NewPM] collapsing nested pass mangers of the same type

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.
Herald added subscribers: wenlei, danielkiss.

ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85676

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

[PATCH] D81865: [clang] Use string tables for static diagnostic descriptions

[David Blaikie via Phabricator via cfe-commits]

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: danielkiss.

Sounds good to me - thanks!


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

https://reviews.llvm.org/D81865

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

[PATCH] D86802: [Modules] Don't parse/load explicit module maps if modules are disabled

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added inline comments.



Comment at: clang/lib/Frontend/FrontendAction.cpp:814
+CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
+*File, /*IsSystem*/ false);
+  else

andrewjcg wrote:
> bruno wrote:
> > Is this clang-formatted? 
> Ahh, yeah.  Should I avoid this sort of thing (and e.g. instead do this in 
> explicit codemod/refactoring diffs)?
It's fine in this case cause it's relevant as part of your changes (indentation 
for the block will change anyways), just double checking on the final 
formatting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86802

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

[Rahman Lavaee via Phabricator via cfe-commits]

rahmanl updated this revision to Diff 288694.
rahmanl added a comment.

- Move getBBAddrMetadata to AsmPrinter as a static function + nits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

Files:
  clang/docs/UsersManual.rst
  clang/test/CodeGen/basic-block-sections.c
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/BasicBlockSections.cpp
  llvm/lib/CodeGen/MIRParser/MIRParser.cpp
  llvm/lib/CodeGen/MachineBasicBlock.cpp
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
  llvm/test/CodeGen/X86/basic-block-sections-labels.ll

Index: llvm/test/CodeGen/X86/basic-block-sections-labels.ll
===
--- llvm/test/CodeGen/X86/basic-block-sections-labels.ll
+++ llvm/test/CodeGen/X86/basic-block-sections-labels.ll
@@ -1,23 +1,24 @@
 ; Check the basic block sections labels option
-; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
 
-define void @_Z3bazb(i1 zeroext) {
-  %2 = alloca i8, align 1
-  %3 = zext i1 %0 to i8
-  store i8 %3, i8* %2, align 1
-  %4 = load i8, i8* %2, align 1
-  %5 = trunc i8 %4 to i1
-  br i1 %5, label %6, label %8
+define void @_Z3bazb(i1 zeroext) personality i32 (...)* @__gxx_personality_v0 {
+  br i1 %0, label %2, label %7
 
-6:; preds = %1
-  %7 = call i32 @_Z3barv()
-  br label %10
+2:
+  %3 = invoke i32 @_Z3barv()
+  to label %7 unwind label %5
+  br label %9
 
-8:; preds = %1
-  %9 = call i32 @_Z3foov()
-  br label %10
+5:
+  landingpad { i8*, i32 }
+  catch i8* null
+  br label %9
 
-10:   ; preds = %8, %6
+7:
+  %8 = call i32 @_Z3foov()
+  br label %9
+
+9:
   ret void
 }
 
@@ -25,9 +26,31 @@
 
 declare i32 @_Z3foov() #1
 
-; LINUX-LABELS: .section
-; LINUX-LABELS: _Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: r.BB._Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: rr.BB._Z3bazb:
+declare i32 @__gxx_personality_v0(...)
+
+; LINUX-LABELS-LABEL:	_Z3bazb:
+; LINUX-LABELS:		.Lfunc_begin0:
+; LINUX-LABELS:		.LBB_END0_[[L1:[0-9]+]]:
+; LINUX-LABELS:		.LBB0_[[L2:[0-9]+]]:
+; LINUX-LABELS:		.LBB_END0_[[L2]]:
+; LINUX-LABELS:		.LBB0_[[L3:[0-9]+]]:
+; LINUX-LABELS:		.LBB_END0_[[L3]]:
+; LINUX-LABELS:		.LBB0_[[L4:[0-9]+]]:
+; LINUX-LABELS:		.LBB_END0_[[L4]]:
+; LINUX-LABELS:		.Lfunc_end0:
+
+; LINUX-LABELS:		.section	.bb_addr_map,"o",@progbits,.text
+; LINUX-LABELS-NEXT:	.quad	.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.byte	4
+; LINUX-LABELS-NEXT:	.uleb128 .Lfunc_begin0-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB_END0_[[L1]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.byte	0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB0_[[L2]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB_END0_[[L2]]-.LBB0_[[L2]]
+; LINUX-LABELS-NEXT:	.byte	0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB0_[[L3]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB_END0_[[L3]]-.LBB0_[[L3]]
+; LINUX-LABELS-NEXT:	.byte	1
+; LINUX-LABELS-NEXT:	.uleb128 .LBB0_[[L4]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB_END0_[[L4]]-.LBB0_[[L4]]
+; LINUX-LABELS-NEXT:	.byte	5
Index: llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
@@ -0,0 +1,35 @@
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -function-sections -basic-block-sections=labels | FileCheck %s
+
+$_Z4fooTIiET_v = comdat any
+
+define dso_local i32 @_Z3barv() {
+  ret i32 0
+}
+;; Check we add SHF_LINK_ORDER for .bb_addr_map and link it with the corresponding .text sections.
+; CHECK:	.section .text._Z3barv,"ax",@progbits
+; CHECK-LABEL:	_Z3barv:
+; CHECK-NEXT:	[[BAR_BEGIN:.+]]:
+; CHECK:	.section .bb_addr_map,"o",@progbits,.text._Z3barv{{$}}
+; CHECK-NEXT:	.quad [[BAR_BEGIN]]
+
+
+define dso_local i32 @_Z3foov() {
+  %1 = call i32 @_Z4fooTIiET_v()
+  ret i32 %1
+}
+; CHECK:	.section .text._Z3foov,"ax",@progbits
+; CHECK-LABEL:	_Z3foov:
+; CHECK-NEXT:	[[FOO_BEGIN:.+]]:
+; CHECK:	.section  .bb_addr_map,"o",@progbits,.text._Z3foov{{$}}
+; CHECK-NEXT:	.quad [[FOO_BEGIN]]
+
+
+define linkonce_odr dso_local i32 @_Z4fooTIiET_v() comdat {
+  ret i32 0
+}
+;; Check we add .bb_addr_map section to a COMDAT group with the corresponding .text section if such a COMDAT exists.
+; CHECK:	.section .text._Z4fooTIiET_v,"axG",@progbits,_Z4fooTIiET_v,comdat
+; CHECK-LABEL: