[PATCH] D145477: run-clang-tidy.py should only search for the clang-apply-replacements if really needed

[Tiwari Abhinav Ashok Kumar via Phabricator via cfe-commits]

aabhinavg created this revision.
aabhinavg added a reviewer: keith.
Herald added subscribers: PiotrZSL, carlosgalvezp.
Herald added a reviewer: njames93.
Herald added a project: All.
aabhinavg requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

run-clang-tidy.py should only search for the clang-apply-replacements if really 
needed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145477

Files:
  clang-tools-extra/clang-tidy/tool/run-clang-tidy.py


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -300,7 +300,7 @@
   build_path)
 
   tmpdir = None
-  if args.fix or (yaml and args.export_fixes):
+  if args.fix:
 clang_apply_replacements_binary = find_binary(
   args.clang_apply_replacements_binary, "clang-apply-replacements",
   build_path)


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -300,7 +300,7 @@
   build_path)
 
   tmpdir = None
-  if args.fix or (yaml and args.export_fixes):
+  if args.fix:
 clang_apply_replacements_binary = find_binary(
   args.clang_apply_replacements_binary, "clang-apply-replacements",
   build_path)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145477: run-clang-tidy.py should only search for the clang-apply-replacements if really needed

[Carlos Galvez via Phabricator via cfe-commits]

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

LGTM, `clang_apply_replacements_binary` is only used in a `if args.fix` block.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145477

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

[PATCH] D145479: [clang][ASTImporter] Import typedefs to distinct records as distinct nodes.

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

balazske created this revision.
Herald added subscribers: steakhal, martong, gamesh411, Szelethus, dkrupp.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: All.
balazske requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When a typedef node is imported, ASTImporter should not find an existing similar
typedef node for it that comes from different context (translation unit or 
scope).
This should avoid a situation where an existing typedef declaration is returned
at import of a typedef, but the underlying type was already imported as a new
type object.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145479

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -8474,6 +8474,81 @@
   ToVaList->getUnderlyingType(), ToBuiltinVaList->getUnderlyingType()));
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportExistingTypedefToRecord) {
+  const char *Code =
+  R"(
+  struct S { int i; };
+  typedef struct S T;
+  extern T x;
+  )";
+  Decl *ToTU = getToTuDecl(Code, Lang_C99);
+  Decl *FromTU = getTuDecl(Code, Lang_C99);
+
+  auto *FromX =
+  FirstDeclMatcher().match(FromTU, varDecl(hasName("x")));
+  auto *ToX = Import(FromX, Lang_C99);
+  EXPECT_TRUE(ToX);
+
+  auto *Typedef1 =
+  FirstDeclMatcher().match(ToTU, typedefDecl(hasName("T")));
+  auto *Typedef2 =
+  LastDeclMatcher().match(ToTU, typedefDecl(hasName("T")));
+  EXPECT_EQ(Typedef1, Typedef2);
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase,
+   ImportExistingTypedefToUnnamedRecord) {
+  const char *Code =
+  R"(
+  typedef const struct { int f; } T;
+  extern T x;
+  )";
+  Decl *ToTU = getToTuDecl(Code, Lang_C99);
+  Decl *FromTU = getTuDecl(Code, Lang_C99);
+
+  auto *FromX =
+  FirstDeclMatcher().match(FromTU, varDecl(hasName("x")));
+  auto *ToX = Import(FromX, Lang_C99);
+  EXPECT_TRUE(ToX);
+
+  auto *Typedef1 =
+  FirstDeclMatcher().match(ToTU, typedefDecl(hasName("T")));
+  auto *Typedef2 =
+  LastDeclMatcher().match(ToTU, typedefDecl(hasName("T")));
+  EXPECT_NE(Typedef1, Typedef2);
+  EXPECT_NE(Typedef1->getUnderlyingType().getTypePtr(),
+Typedef2->getUnderlyingType().getTypePtr());
+  EXPECT_EQ(ToX->getType()->getAs()->getDecl(), Typedef2);
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, ImportTwoTypedefsToUnnamedRecord) {
+  const char *Code =
+  R"(
+  typedef struct { int f; } T1;
+  typedef struct { int f; } T2;
+  extern T1 x1;
+  extern T2 x2;
+  )";
+  Decl *ToTU = getToTuDecl("", Lang_C99);
+  Decl *FromTU = getTuDecl(Code, Lang_C99);
+
+  auto *FromX1 =
+  FirstDeclMatcher().match(FromTU, varDecl(hasName("x1")));
+  auto *FromX2 =
+  FirstDeclMatcher().match(FromTU, varDecl(hasName("x2")));
+  auto *ToX1 = Import(FromX1, Lang_C99);
+  EXPECT_TRUE(ToX1);
+  auto *ToX2 = Import(FromX2, Lang_C99);
+  EXPECT_TRUE(ToX2);
+
+  auto *Typedef1 =
+  FirstDeclMatcher().match(ToTU, typedefDecl(hasName("T1")));
+  auto *Typedef2 =
+  FirstDeclMatcher().match(ToTU, typedefDecl(hasName("T2")));
+  EXPECT_NE(Typedef1->getUnderlyingType().getTypePtr(),
+Typedef2->getUnderlyingType().getTypePtr());
+}
+
 INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ASTImporterLookupTableTest,
  DefaultTestValuesForRunOptions);
 
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -2509,6 +2509,22 @@
 QualType FromUT = D->getUnderlyingType();
 QualType FoundUT = FoundTypedef->getUnderlyingType();
 if (Importer.IsStructurallyEquivalent(FromUT, FoundUT)) {
+  // If the underlying declarations are unnamed records these can be
+  // imported as different types. We should create a distinct typedef
+  // node in this case.
+  // If we found an existing underlying type with a record in a
+  // different context (than the imported), this is already reason for
+  // having distinct typedef nodes for these.
+  // Again this can create situation like
+  // 'typedef int T; typedef int T;' but this is hard to avoid without
+  // a rename strategy at import.
+  if (!FromUT.isNull() && !FoundUT.isNull()) {
+RecordDecl *FromR = FromUT->getAsRecordDecl();
+RecordDecl *FoundR = FoundUT->getAsRecordDecl();
+if (FromR && FoundR &&
+!hasSameVisibilityContextAndLinkage(FoundR, FromR))
+  continue;
+  }
   // If the "From" context has a complete underlying type but we
   // already

[PATCH] D143971: [clang-tidy] Flag more buggy string constructor cases

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp:97
+  const auto CharToIntCastExpr = implicitCastExpr(
+  hasSourceExpression(expr(hasType(qualType(isAnyCharacter(),
+  hasImplicitDestinationType(NonCharacterInteger));

PiotrZSL wrote:
> reuse here CharExpr 
try with test like this:

```
struct Class
{
   operator char() const;
};

Class c;
std::string value(c, 5);
```

I fear that hasSourceExpression could match Class type, and therefore this case 
wouldn't be detected.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143971

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

[PATCH] D130303: Handle template parameter-dependent bit field widths in libclang

[Igor Kushnir via Phabricator via cfe-commits]

vedgy added inline comments.



Comment at: clang/include/clang-c/Index.h:3552
+ * If the cursor does not reference a bit field declaration or if the bit
+ * field's width does not depend on template parameters, 0 is returned.
+ */

collinbaker wrote:
> vedgy wrote:
> > I just thought how the new API could be used in KDevelop. Currently when 
> > `clang_getFieldDeclBitWidth()` is positive, e.g. 2, KDevelop shows  ` : 2` 
> > after the data member name in a tooltip. Ideally a template-param-dependent 
> > expression (actual code) would be displayed after the colon. If that's 
> > difficult to implement, `: [tparam-dependent]` or `: ?` could be displayed 
> > instead. But it would be more convenient and efficient to get this 
> > information by a single call to `clang_getFieldDeclBitWidth()` instead of 
> > calling `clang_isFieldDeclBitWidthDependent()` each time 
> > `clang_getFieldDeclBitWidth()` returns `-1`. So how about returning `-2` or 
> > `0` from `clang_getFieldDeclBitWidth()` instead of adding this new API?
> I understand the motivation but I don't think requiring an extra call is 
> asking too much of libclang clients, and it's one extra call that doesn't do 
> much work and will be called rarely so I don't see efficiency concerns. 
> Without strong reasons otherwise I think it's better to be explicit here.
KDevelop calls `clang_getFieldDeclBitWidth()` for each encountered class member 
declaration. `clang_isFieldDeclBitWidthDependent()` would have to be called 
each time `clang_getFieldDeclBitWidth()` returns `-1`, which would be most of 
the time, because few class members are bit-fields. The work this new function 
does is the same as that of `clang_getFieldDeclBitWidth()`  (repeated).

If the concern about returning `-2` from `clang_getFieldDeclBitWidth()` is 
cryptic return codes, an `enum` with named constants can be introduced.

If the concern is breaking backward compatibility for users that relied on the 
returned value being positive or `-1`, then a replacement for 
`clang_getFieldDeclBitWidth()` with the most convenient API should be 
introduced and `clang_getFieldDeclBitWidth()` itself - deprecated.

KDevelop simply stores the value returned by `clang_getFieldDeclBitWidth()` in 
an `int16_t m_bitWidth` data member and uses it later. So if `-2` is returned, 
the only place in code to adjust would be the use of this data member. With the 
current `clang_isFieldDeclBitWidthDependent()` implementation, this function 
would have to be called, `-2` explicitly stored in `m_bitWidth` and the use of 
`m_bitWidth` would have to be adjusted just the same.

Have you considered potential usage of the added API in your project? Which 
alternative would be more convenient to use?



Comment at: clang/tools/libclang/CXType.cpp:13
 
+#include "CXType.h"
 #include "CIndexer.h"

I guess //clang-format// did this include reordering. But it certainly looks 
out of place and the include order becomes wrong. So I think it should be 
reverted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130303

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

[clang-tools-extra] 144e236 - run-clang-tidy.py should only search for the clang-apply-replacements if really needed

[via cfe-commits]

Author: Anonymous
Date: 2023-03-07T14:52:34+05:30
New Revision: 144e2364410cc57e919eee38402bc5347f1d57b8

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

LOG: run-clang-tidy.py should only search for the clang-apply-replacements if 
really needed

run-clang-tidy.py should only search for the clang-apply-replacements if really 
needed.

Reviewed By: carlosgalvezp

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/tool/run-clang-tidy.py

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py 
b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
index e3da6fb9b096..30e29182908d 100755
--- a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -300,7 +300,7 @@ def main():
   build_path)
 
   tmpdir = None
-  if args.fix or (yaml and args.export_fixes):
+  if args.fix:
 clang_apply_replacements_binary = find_binary(
   args.clang_apply_replacements_binary, "clang-apply-replacements",
   build_path)



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

[PATCH] D145477: run-clang-tidy.py should only search for the clang-apply-replacements if really needed

[Tiwari Abhinav Ashok Kumar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG144e2364410c: run-clang-tidy.py should only search for the 
clang-apply-replacements if really… (authored by Anonymous, committed by 
aabhinavg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145477

Files:
  clang-tools-extra/clang-tidy/tool/run-clang-tidy.py


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -300,7 +300,7 @@
   build_path)
 
   tmpdir = None
-  if args.fix or (yaml and args.export_fixes):
+  if args.fix:
 clang_apply_replacements_binary = find_binary(
   args.clang_apply_replacements_binary, "clang-apply-replacements",
   build_path)


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -300,7 +300,7 @@
   build_path)
 
   tmpdir = None
-  if args.fix or (yaml and args.export_fixes):
+  if args.fix:
 clang_apply_replacements_binary = find_binary(
   args.clang_apply_replacements_binary, "clang-apply-replacements",
   build_path)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142872: Honor the fwrapv option when generating the inbounds GEP .

[Nikita Popov via Phabricator via cfe-commits]

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

The original issue has been fixed by the mentioned LLVM changes. As such, I 
don't think we need to actually do anything on the Clang side. Please let me 
know if you're still seeing issues in some case...


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

https://reviews.llvm.org/D142872

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

[clang] fb30904 - [UTC] Enable --function-signature by default

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2023-03-07T10:27:52+01:00
New Revision: fb309041f0c37fa2798305ae02cf6910bf0b402b

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

LOG: [UTC] Enable --function-signature by default

This patch enables --function-signature by default under --version 2
and makes --version 2 the default. This means that all newly created
tests will check the function signature, while leaving old tests alone.

There's two motivations for this change:

* Without --function-signature, the generated check lines may fail
  in a very hard to understand way if the test both includes a
  function definition and a call to that function. (Though we could
  address this by making the CHECK-LABEL stricter, without checking
  the full signature.)
* This actually checks that uses of the arguments in the function
  body use the correct argument, instead of matching against any
  variable.

This is a replacement for D139006 and D140212 based on the
--version mechanism.

I did not include an opt-out flag --no-function-signature because
I'm not sure we need it. Would be happy to include it though,
if desired.

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

Added: 
clang/test/utils/update_cc_test_checks/Inputs/mangled_names.c.v2.expected

llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/basic.ll.v2.expected

Modified: 

clang/test/utils/update_cc_test_checks/Inputs/mangled_names.c.funcsig.v2.expected
clang/test/utils/update_cc_test_checks/mangled_names.test
llvm/test/tools/UpdateTestChecks/update_test_checks/basic.test
llvm/utils/UpdateTestChecks/common.py

Removed: 




diff  --git 
a/clang/test/utils/update_cc_test_checks/Inputs/mangled_names.c.funcsig.v2.expected
 
b/clang/test/utils/update_cc_test_checks/Inputs/mangled_names.c.funcsig.v2.expected
index c70179fc21777..636f7af14d060 100644
--- 
a/clang/test/utils/update_cc_test_checks/Inputs/mangled_names.c.funcsig.v2.expected
+++ 
b/clang/test/utils/update_cc_test_checks/Inputs/mangled_names.c.funcsig.v2.expected
@@ -1,4 +1,4 @@
-// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py 
UTC_ARGS: --function-signature --version 2
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py 
UTC_ARGS: --version 2
 // Example input for update_cc_test_checks
 // RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s | 
FileCheck %s
 

diff  --git 
a/clang/test/utils/update_cc_test_checks/Inputs/mangled_names.c.v2.expected 
b/clang/test/utils/update_cc_test_checks/Inputs/mangled_names.c.v2.expected
new file mode 100644
index 0..636f7af14d060
--- /dev/null
+++ b/clang/test/utils/update_cc_test_checks/Inputs/mangled_names.c.v2.expected
@@ -0,0 +1,43 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py 
UTC_ARGS: --version 2
+// Example input for update_cc_test_checks
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s | 
FileCheck %s
+
+// CHECK-LABEL: define dso_local i64 @test
+// CHECK-SAME: (i64 noundef [[A:%.*]], i32 noundef [[B:%.*]]) 
#[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[A_ADDR:%.*]] = alloca i64, align 8
+// CHECK-NEXT:[[B_ADDR:%.*]] = alloca i32, align 4
+// CHECK-NEXT:store i64 [[A]], ptr [[A_ADDR]], align 8
+// CHECK-NEXT:store i32 [[B]], ptr [[B_ADDR]], align 4
+// CHECK-NEXT:[[TMP0:%.*]] = load i64, ptr [[A_ADDR]], align 8
+// CHECK-NEXT:[[TMP1:%.*]] = load i32, ptr [[B_ADDR]], align 4
+// CHECK-NEXT:[[CONV:%.*]] = sext i32 [[TMP1]] to i64
+// CHECK-NEXT:[[ADD:%.*]] = add nsw i64 [[TMP0]], [[CONV]]
+// CHECK-NEXT:ret i64 [[ADD]]
+//
+long test(long a, int b) {
+  return a + b;
+}
+
+// A function with a mangled name
+// CHECK-LABEL: define dso_local i64 @_Z4testlii
+// CHECK-SAME: (i64 noundef [[A:%.*]], i32 noundef [[B:%.*]], i32 noundef 
[[C:%.*]]) #[[ATTR0]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[A_ADDR:%.*]] = alloca i64, align 8
+// CHECK-NEXT:[[B_ADDR:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[C_ADDR:%.*]] = alloca i32, align 4
+// CHECK-NEXT:store i64 [[A]], ptr [[A_ADDR]], align 8
+// CHECK-NEXT:store i32 [[B]], ptr [[B_ADDR]], align 4
+// CHECK-NEXT:store i32 [[C]], ptr [[C_ADDR]], align 4
+// CHECK-NEXT:[[TMP0:%.*]] = load i64, ptr [[A_ADDR]], align 8
+// CHECK-NEXT:[[TMP1:%.*]] = load i32, ptr [[B_ADDR]], align 4
+// CHECK-NEXT:[[CONV:%.*]] = sext i32 [[TMP1]] to i64
+// CHECK-NEXT:[[ADD:%.*]] = add nsw i64 [[TMP0]], [[CONV]]
+// CHECK-NEXT:[[TMP2:%.*]] = load i32, ptr [[C_ADDR]], align 4
+// CHECK-NEXT:[[CONV1:%.*]] = sext i32 [[TMP2]] to i64
+// CHECK-NEXT:[[ADD2:%.*]] = add nsw i64 [[ADD]], [[CONV1]]
+// CHECK-NEXT:ret i64 [[ADD2]]
+//
+__attribute__(

[PATCH] D145149: [UTC] Enable --function-signature by default

[Nikita Popov 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 rGfb309041f0c3: [UTC] Enable --function-signature by default 
(authored by nikic).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D145149?vs=502650&id=502952#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145149

Files:
  
clang/test/utils/update_cc_test_checks/Inputs/mangled_names.c.funcsig.v2.expected
  clang/test/utils/update_cc_test_checks/Inputs/mangled_names.c.v2.expected
  clang/test/utils/update_cc_test_checks/mangled_names.test
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/basic.ll.v2.expected
  llvm/test/tools/UpdateTestChecks/update_test_checks/basic.test
  llvm/utils/UpdateTestChecks/common.py

Index: llvm/utils/UpdateTestChecks/common.py
===
--- llvm/utils/UpdateTestChecks/common.py
+++ llvm/utils/UpdateTestChecks/common.py
@@ -22,9 +22,10 @@
 Version changelog:
 
 1: Initial version, used by tests that don't specify --version explicitly.
-2: --function-signature now also checks return type/attributes.
+2: --function-signature is now enabled by default and also checks return
+   type/attributes.
 """
-DEFAULT_VERSION = 1
+DEFAULT_VERSION = 2
 
 class Regex(object):
   """Wrap a compiled regular expression object to allow deep copy of a regexp.
@@ -157,6 +158,11 @@
   _global_hex_value_regex = args.global_hex_value_regex
   return args
 
+def parse_args(parser, argv):
+  args = parser.parse_args(argv)
+  if args.version >= 2:
+args.function_signature = True
+  return args
 
 class InputLineInfo(object):
   def __init__(self, line, line_number, args, argv):
@@ -246,7 +252,7 @@
   if not is_regenerate:
 argv.insert(1, '--version=' + str(DEFAULT_VERSION))
 
-  args = parser.parse_args(argv[1:])
+  args = parse_args(parser, argv[1:])
   if argparse_callback is not None:
 argparse_callback(args)
   if is_regenerate:
@@ -1199,6 +1205,8 @@
 continue
 if parser.get_default(action.dest) == value:
   continue  # Don't add default values
+if action.dest == 'function_signature' and args.version >= 2:
+  continue # Enabled by default in version 2
 if action.dest == 'filters':
   # Create a separate option for each filter element.  The value is a list
   # of Filter objects.
@@ -1225,7 +1233,7 @@
 for option in shlex.split(cmd_m.group('cmd').strip()):
   if option:
 argv.append(option)
-args = parser.parse_args(filter(lambda arg: arg not in args.tests, argv))
+args = parse_args(parser, filter(lambda arg: arg not in args.tests, argv))
 if argparse_callback is not None:
   argparse_callback(args)
   return args, argv
Index: llvm/test/tools/UpdateTestChecks/update_test_checks/basic.test
===
--- llvm/test/tools/UpdateTestChecks/update_test_checks/basic.test
+++ llvm/test/tools/UpdateTestChecks/update_test_checks/basic.test
@@ -14,3 +14,8 @@
 ## added to the update invocation below.
 # RUN: %update_test_checks %t.ll
 # RUN: diff -u %t.ll %S/Inputs/basic.ll.funcsig.expected
+## Restore the original file without --function-signature and check that
+## --version 2 will implicitly enable it and also check the return type.
+# RUN: cp -f %S/Inputs/basic.ll %t.ll
+# RUN: %update_test_checks %t.ll --version 2
+# RUN: diff -u %t.ll %S/Inputs/basic.ll.v2.expected
Index: llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/basic.ll.v2.expected
===
--- /dev/null
+++ llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/basic.ll.v2.expected
@@ -0,0 +1,65 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 2
+; Example input for update_test_checks (taken from test/Transforms/InstSimplify/add.ll)
+; RUN: opt < %s -passes=instsimplify -S | FileCheck %s
+
+define i32 @common_sub_operand(i32 %X, i32 %Y) {
+; CHECK-LABEL: define i32 @common_sub_operand
+; CHECK-SAME: (i32 [[X:%.*]], i32 [[Y:%.*]]) {
+; CHECK-NEXT:ret i32 [[X]]
+;
+  %Z = sub i32 %X, %Y
+  %Q = add i32 %Z, %Y
+  ret i32 %Q
+}
+
+define i32 @negated_operand(i32 %x) {
+; CHECK-LABEL: define i32 @negated_operand
+; CHECK-SAME: (i32 [[X:%.*]]) {
+; CHECK-NEXT:ret i32 0
+;
+  %negx = sub i32 0, %x
+  %r = add i32 %negx, %x
+  ret i32 %r
+}
+
+define <2 x i32> @negated_operand_commute_vec(<2 x i32> %x) {
+; CHECK-LABEL: define <2 x i32> @negated_operand_commute_vec
+; CHECK-SAME: (<2 x i32> [[X:%.*]]) {
+; CHECK-NEXT:ret <2 x i32> zeroinitializer
+;
+  %negx = sub <2 x i32> zeroinitializer, %x
+  %r = add <2 x i32> %x, %negx
+  ret <2 x i32> %r
+}
+
+define i8 @knownnegation(i8 %x, i8 %

[PATCH] D145238: [NVPTX] Expose LDU builtins

[Jakub Chlanda via Phabricator via cfe-commits]

jchlanda updated this revision to Diff 502948.
jchlanda marked 3 inline comments as done.
jchlanda added a comment.

Address PR comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145238

Files:
  clang/include/clang/Basic/BuiltinsNVPTX.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-nvptx-native-half-type-err.c
  clang/test/CodeGen/builtins-nvptx-native-half-type.c
  clang/test/CodeGen/builtins-nvptx.c
  llvm/test/CodeGen/NVPTX/ldu-ldg.ll

Index: llvm/test/CodeGen/NVPTX/ldu-ldg.ll
===
--- llvm/test/CodeGen/NVPTX/ldu-ldg.ll
+++ llvm/test/CodeGen/NVPTX/ldu-ldg.ll
@@ -3,7 +3,13 @@
 
 
 declare i8 @llvm.nvvm.ldu.global.i.i8.p1(ptr addrspace(1) %ptr, i32 %align)
+declare i16 @llvm.nvvm.ldu.global.i.i16.p1(ptr addrspace(1) %ptr, i32 %align)
 declare i32 @llvm.nvvm.ldu.global.i.i32.p1(ptr addrspace(1) %ptr, i32 %align)
+declare i64 @llvm.nvvm.ldu.global.i.i64.p1(ptr addrspace(1) %ptr, i32 %align)
+declare float @llvm.nvvm.ldu.global.f.f32.p1(ptr addrspace(1) %ptr, i32 %align)
+declare double @llvm.nvvm.ldu.global.f.f64.p1(ptr addrspace(1) %ptr, i32 %align)
+declare half @llvm.nvvm.ldu.global.f.f16.p1(ptr addrspace(1) %ptr, i32 %align)
+declare <2 x half> @llvm.nvvm.ldu.global.f.v2f16.p1(ptr addrspace(1) %ptr, i32 %align)
 
 declare i8 @llvm.nvvm.ldg.global.i.i8.p1(ptr addrspace(1) %ptr, i32 %align)
 declare i16 @llvm.nvvm.ldg.global.i.i16.p1(ptr addrspace(1) %ptr, i32 %align)
@@ -16,70 +22,112 @@
 
 ; CHECK: test_ldu_i8
 define i8 @test_ldu_i8(ptr addrspace(1) %ptr) {
-; ldu.global.u8
+  ; CHECK: ldu.global.u8
   %val = tail call i8 @llvm.nvvm.ldu.global.i.i8.p1(ptr addrspace(1) %ptr, i32 4)
   ret i8 %val
 }
 
+; CHECK: test_ldu_i16
+define i16 @test_ldu_i16(ptr addrspace(1) %ptr) {
+  ; CHECK: ldu.global.u16
+  %val = tail call i16 @llvm.nvvm.ldu.global.i.i16.p1(ptr addrspace(1) %ptr, i32 2)
+  ret i16 %val
+}
+
 ; CHECK: test_ldu_i32
 define i32 @test_ldu_i32(ptr addrspace(1) %ptr) {
-; ldu.global.u32
+  ; CHECK: ldu.global.u32
   %val = tail call i32 @llvm.nvvm.ldu.global.i.i32.p1(ptr addrspace(1) %ptr, i32 4)
   ret i32 %val
 }
 
+; CHECK: test_ldu_i64
+define i64 @test_ldu_i64(ptr addrspace(1) %ptr) {
+  ; CHECK: ldu.global.u64
+  %val = tail call i64 @llvm.nvvm.ldu.global.i.i64.p1(ptr addrspace(1) %ptr, i32 8)
+  ret i64 %val
+}
+
+; CHECK: test_ldu_f32
+define float @test_ldu_f32(ptr addrspace(1) %ptr) {
+  ; CHECK: ldu.global.f32
+  %val = tail call float @llvm.nvvm.ldu.global.f.f32.p1(ptr addrspace(1) %ptr, i32 4)
+  ret float %val
+}
+
+; CHECK: test_ldu_f64
+define double @test_ldu_f64(ptr addrspace(1) %ptr) {
+  ; CHECK: ldu.global.f64
+  %val = tail call double @llvm.nvvm.ldu.global.f.f64.p1(ptr addrspace(1) %ptr, i32 8)
+  ret double %val
+}
+
+; CHECK: test_ldu_f16
+define half @test_ldu_f16(ptr addrspace(1) %ptr) {
+  ; CHECK: ldu.global.b16
+  %val = tail call half @llvm.nvvm.ldu.global.f.f16.p1(ptr addrspace(1) %ptr, i32 2)
+  ret half %val
+}
+
+; CHECK: test_ldu_v2f16
+define <2 x half> @test_ldu_v2f16(ptr addrspace(1) %ptr) {
+  ; CHECK: ldu.global.b32
+  %val = tail call <2 x half> @llvm.nvvm.ldu.global.f.v2f16.p1(ptr addrspace(1) %ptr, i32 4)
+  ret <2 x half> %val
+}
+
 ; CHECK: test_ldg_i8
 define i8 @test_ldg_i8(ptr addrspace(1) %ptr) {
-; ld.global.nc.u8
+  ; CHECK: ld.global.nc.u8
   %val = tail call i8 @llvm.nvvm.ldg.global.i.i8.p1(ptr addrspace(1) %ptr, i32 4)
   ret i8 %val
 }
 
 ; CHECK: test_ldg_i16
 define i16 @test_ldg_i16(ptr addrspace(1) %ptr) {
-; ld.global.nc.u16
-  %val = tail call i16 @llvm.nvvm.ldg.global.i.i16.p1(ptr addrspace(1) %ptr, i32 4)
+  ; CHECK: ld.global.nc.u16
+  %val = tail call i16 @llvm.nvvm.ldg.global.i.i16.p1(ptr addrspace(1) %ptr, i32 2)
   ret i16 %val
 }
 
 ; CHECK: test_ldg_i32
 define i32 @test_ldg_i32(ptr addrspace(1) %ptr) {
-; ld.global.nc.u32
+  ; CHECK: ld.global.nc.u32
   %val = tail call i32 @llvm.nvvm.ldg.global.i.i32.p1(ptr addrspace(1) %ptr, i32 4)
   ret i32 %val
 }
 
 ; CHECK: test_ldg_i64
 define i64 @test_ldg_i64(ptr addrspace(1) %ptr) {
-; ld.global.nc.u64
+  ; CHECK: ld.global.nc.u64
   %val = tail call i64 @llvm.nvvm.ldg.global.i.i64.p1(ptr addrspace(1) %ptr, i32 8)
   ret i64 %val
 }
 
 ; CHECK: test_ldg_f32
 define float @test_ldg_f32(ptr addrspace(1) %ptr) {
-; ld.global.nc.u64
+  ; CHECK: ld.global.nc.f32
   %val = tail call float @llvm.nvvm.ldg.global.f.f32.p1(ptr addrspace(1) %ptr, i32 4)
   ret float %val
 }
 
 ; CHECK: test_ldg_f64
 define double @test_ldg_f64(ptr addrspace(1) %ptr) {
-; ld.global.nc.u64
+  ; CHECK: ld.global.nc.f64
   %val = tail call double @llvm.nvvm.ldg.global.f.f64.p1(ptr addrspace(1) %ptr, i32 8)
   ret double %val
 }
 
 ; CHECK: test_ldg_f16
 define half @test_ldg_f16(ptr addrspace(1) %ptr) {
-; ld.global.nc.b16
-  %val = tail call half @llvm.nvvm.ldg.global.f.f16.p1(ptr addrspace(1) %ptr, i32 4)
+  ; CHECK: ld.global.nc.b16
+  

[PATCH] D145238: [NVPTX] Expose LDU builtins

[Jakub Chlanda via Phabricator via cfe-commits]

jchlanda added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:18116
+case NVPTX::BI__nvvm_ldu_h:
+  BuiltinName = "__nvvm_ldu_h";
+  break;

tra wrote:
> Can we use the standard `StringRef Name = 
> getContext().BuiltinInfo.getName(BuiltinID);` to figure out the builtin name?
Ha, had a feeling it would exist, couldn't find it. Thanks.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:18261-18263
+  std::string ErrMsg{HalfSupport.second};
+  CGM.Error(E->getExprLoc(),
+ErrMsg.append(" requires native half type support.").c_str());

tra wrote:
> Nit: this would be a bit more readable:
> ```
> std::string BuiltinName{HalfSupport.second};
> CGM.Error(E->getExprLoc(),  BuiltinName + " requires native half type 
> support.")` 
> ```
> You may also consider changing returned `BuiltinName` to be `std::string`, so 
> you would not need an explicit temp var. 
Done the `std::string` return, thanks.



Comment at: llvm/test/CodeGen/NVPTX/ldu-ldg.ll:60
+define double @test_ldu_f64(ptr addrspace(1) %ptr) {
+; ldu.global.u64
+  %val = tail call double @llvm.nvvm.ldu.global.f.f64.p1(ptr addrspace(1) 
%ptr, i32 8)

tra wrote:
> Hmm. I wonder why we end up with `u64` here and not `b64`. Not that it 
> matters in this case, but it is a discrepancy vs. `f16`.
That is copy/paste sloppiness on my part, sorry. I've updated the test to check 
generated PTX, not just the labels, and fixed the values.

It generates correct kinds of loads, based on the type, the only discrepancy is 
that it doesn't distinguish between signed and unsigned loads, always choosing 
the unsigned variant. I think that's by design, at 
[ISel](https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp#L1683)
 there is a check if the load needs to be extended and a correct `CVT` 
instruction will be added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145238

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

[PATCH] D145441: [AMDGPU] Define data layout entries for buffers

[Jay Foad via Phabricator via cfe-commits]

foad added a comment.

Just my 2p: it feels a bit premature to commit patches for this. It feels more 
like something you could prototype on a branch somewhere and come back when you 
have more experience with how it all works out in practice.

But I don't actually object to the patch, if the other reviewers are happy and 
it doens't break anything.

> The first is address space 7, a non-integral address space (which was
> already in the data layout) that has 160-bit pointers (which are
> 256-bit aligned)

Any particular reason for choosing 256-bit alignment?

> However, they must not be used as the arguments to
> getelementptr or otherwise used in address computations

I don't understand what kind of rule this is and how it would be enforced. Is 
it something that will be written into the IR LangRef?

> This commit also updates the "fallback address space" for buffer
> intrinsics to the buffer resource,

It's not clear to me that this is any more or less correct, since 7 and 8 
behave identically wrt alias analysis don't they?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145441

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

[PATCH] D136919: [X86][RFC] Change mangle name of __bf16 from u6__bf16 to DF16b

[Ties Stuij via Phabricator via cfe-commits]

stuij added a comment.

FWIW, at Arm we decided to keep the old name mangling to minimise friction with 
existing code/libraries, but allow more operations with this same 
name-mangling. We also discussed with Red Hat and they were ok with this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136919

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

[PATCH] D143418: [libclang] Add API to override preamble storage path

[Igor Kushnir via Phabricator via cfe-commits]

vedgy added a comment.

In D143418#4172587 , @aaron.ballman 
wrote:

> Thank you, this LGTM! I have to head out shortly, so I'll land this on your 
> behalf tomorrow when I have the time to babysit the postcommit build farm. 
> However, if you'd like to request commit access for yourself, I think that'd 
> be reasonable as well: 
> https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access Let me 
> know which route you'd prefer going.

https://llvm.org/docs/Phabricator.html#committing-a-change says:

> Using the Arcanist tool can simplify the process of committing reviewed code 
> as it will retrieve reviewers, the Differential Revision, etc from the review 
> and place it in the commit message. You may also commit an accepted change 
> directly using git push, per the section in the getting started guide.

But how to use the Arcanist tool to push reviewed changes is not elaborated. As 
far as I can tell from the Arcanist documentation, if I have the commit in my 
local //main// branch which is currently checked out, I simply need to run `arc 
land` without arguments. Hopefully the Git pre-push hook 
 I have set up will 
run in this case.

I have no idea how to babysit the postcommit build farm, and so wouldn't commit 
when you don't have time anyway. I plan to make only one more change to 
libclang in the foreseeable future, so not sure learning to handle postcommit 
issues is justified. I'll leave this to your discretion as I have no idea how 
difficult and time-consuming this work is, compared to learning how to do it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143418

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

[PATCH] D143984: [DebugMetadata] Simplify handling subprogram's retainedNodes field. NFCI (1/7)

[Kristina Bessonova via Phabricator via cfe-commits]

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



Comment at: llvm/include/llvm/IR/DIBuilder.h:76
 
-/// Each subprogram's preserved labels.
-DenseMap> PreservedLabels;
+SmallVectorImpl &
+getSubprogramNodesTrackingVector(const DIScope *S) {

aprantl wrote:
> Do you need a writeable copy, or could you get by with returning an 
> `ArrayRef`?
It should return smth suitable to emplace new elements, so, right, we need a 
writable reference here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143984

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

[PATCH] D145228: [clangd] Add clangd headers to install targets

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

i agree with Sam's concerns here. clangd isn't designed to be consumed as a 
library, but rather as a binary through LSP. increasing surface are here and 
letting people build applications on top of clangd internals would create extra 
maintenance burden that we're not equipped to support.

do you have any specific use case that's pending on this change, or is this a 
change that might allow things in the future? if you have a specific need it 
would be better if you can share it directly and we can look for other 
solutions instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145228

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

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added inline comments.



Comment at: clang/test/Driver/flang/flang-omp.f90:1
+! Check that flang -fc1 is invoked when in --driver-mode=flang 
+! and the relevant openmp and openmp offload flags are utilised

agozillon wrote:
> awarzynski wrote:
> > agozillon wrote:
> > > awarzynski wrote:
> > > > agozillon wrote:
> > > > > agozillon wrote:
> > > > > > awarzynski wrote:
> > > > > > > This test looks correct to me, but please note that:
> > > > > > > 
> > > > > > > ```
> > > > > > > ! Check that flang -fc1 is invoked when in --driver-mode=flang 
> > > > > > > ```
> > > > > > > 
> > > > > > > yet (`%clang` instead of `%flang`)
> > > > > > > 
> > > > > > > ```
> > > > > > > ! RUN: %clang --driver-mode=flang -### -fopenmp %s 2>&1 | 
> > > > > > > FileCheck --check-prefixes=CHECK-OPENMP %s
> > > > > > > ```
> > > > > > > 
> > > > > > > I'm not really sure whether we should be testing Flang-specific 
> > > > > > > logic in Clang. Having said that, Flang does use `clangDriver` to 
> > > > > > > implement its driver :) 
> > > > > > > 
> > > > > > > You could consider using 
> > > > > > > https://github.com/llvm/llvm-project/blob/main/flang/test/Driver/frontend-forwarding.f90
> > > > > > >  instead (or add an OpenMP specific file there).
> > > > > > > 
> > > > > > > Not a blocker.
> > > > > > Yes, I wasn't so sure either as it felt a little weird to test 
> > > > > > Flang components inside of Clang, but as you said it is a Clang 
> > > > > > toolchain (that this test is checking) and borrows from the 
> > > > > > clangDriver! 
> > > > > > 
> > > > > > I borrowed this test from other similar tests in the same folder 
> > > > > > that test other flang specific driver logic in a similar manner, 
> > > > > > but I am more than happy to add an additional flang specific driver 
> > > > > > test as you mention! 
> > > > > On further looking into the frontend-forwarding test, I don't know if 
> > > > > it is suitable for fopenmp-is-device as it is an fc1 option and won't 
> > > > > be forwarded from the flang-new frontend down to fc1 at the moment! 
> > > > > 
> > > > > I think this test will be more suitable when additional flags like 
> > > > > the fopenmp-targets (or similar flags) that are used in this test are 
> > > > > added to the Flang driver. As they spawn/propagate the 
> > > > > openmp-is-device flag. However, perhaps I am incorrect.
> > > > > On further looking into the frontend-forwarding test, I don't know if 
> > > > > it is suitable for fopenmp-is-device as it is an fc1 option and won't 
> > > > > be forwarded from the flang-new frontend down to fc1 at the moment!
> > > > 
> > > > It should be - that's what the logic in Flang.cpp does, no? And if it 
> > > > doesn't, is it a deliberate design decision?
> > > I believe the logic in Flang.h/Flang.cpp just invokes for -fc1 not the 
> > > Frontend driver, at least that's what it looks like from the ConstructJob 
> > > task that Clang invokes. 
> > > 
> > > It's a deliberate design decision that -fopenmp-is-device is an fc1 
> > > option, it mimics the way Clang currently does it, which is a cc1 option, 
> > > there is no way to directly specify it to clang without -cc1 I think. The 
> > > hope is to keep the Flang OpenMP flags as similar to Clang's as possible 
> > > for the time being I believe.
> > > I believe the logic in Flang.h/Flang.cpp just invokes for -fc1 not the 
> > > Frontend driver.
> > 
> > `flang-new -fc1` **is** the frontend driver ;-) So:
> > * you've added the  logic to forward `-fopenmp-is-device` that should work 
> > for both `flang-new` and `clang --driver-mode=flang`, but
> > * you're only testing one of these.
> > 
> > There should have tests for both. In particular for `flang-new` given that 
> > this patch adds new functionality to LLVM Flang. If that doesn't work, 
> > let's try to figure it out together.
> > 
> > > The hope is to keep the Flang OpenMP flags as similar to Clang's as 
> > > possible for the time being I believe.
> > 
> > +1 We should try as much as we can to keep Clang's and Flang's behavior 
> > consistent :) (which you do 👍🏻 )
> Ah I didn't realise that, thank you very much for clarifying that for me! :) 
> I thought it was bypassing it with -fc1, and yjsy flang-new without it was 
> the frontend! I still have a lot to learn, so I apologies for the 
> misunderstanding! 
> 
> In this case do you mean test with "flang-new -fc1 -fopenmp-is-device" or 
> "flang-new -fopenmp-is-device", as the latter will not be accepted by 
> flang-new in the current patch but I do test the -fc1 variation in the 
> omp-is-device.f90 test which is more about testing the attribute admittedly 
> as opposed to the driver command. 
> 
> I don't know if testing it in the frontend-forwarding.f90 test is the right 
> location in this case as it doesn't test with -fc1, it seems to test options 
> are forwarded to -fc1 correctly, I could be incorrect though. In future 
> passing an argument like -fopenm

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

[Kiran Chandramohan via Phabricator via cfe-commits]

kiranchandramohan added a comment.

Thanks for the update and the replies. See comments inline.




Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1561
+  /// should be placed.
+  /// \param HasRegion True if the op has a region associated with it, false
+  /// otherwise

TIFitis wrote:
> kiranchandramohan wrote:
> > Can't the presence/absence of the BodyGenCB indicate the presence/absence 
> > of a region?
> The `BodyGenCB` is always present as an argument right? Passing a `nullptr` 
> when its not required doesn't seem possible at least when using the 
> `BodyGenCallbackTy`. Is there a way to selectively pass the `BodyGenCB`?
The only optional CallBack seems to be the `FinalizeCallbackTy`. It is defined 
as an `std::function` whereas `BodyGenCallBackTy` is defined as a 
`function_ref`. But the definition of `function_ref` looks like it can be used 
to check whether it is a `nullptr` or not. Did you face any issues in trying to 
make it optional with a default parameter value of `nullptr`?

https://llvm.org/doxygen/STLFunctionalExtras_8h_source.html#l00036

```
  void emitCancelationCheckImpl(Value *CancelFlag,
omp::Directive CanceledDirective,
FinalizeCallbackTy ExitCB = {});
```



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4052-4060
+  // LLVM utilities like blocks with terminators.
+  auto *UI = Builder.CreateUnreachable();
+  Instruction *ThenTI = UI, *ElseTI = nullptr;
+  if (IfCond) {
+SplitBlockAndInsertIfThenElse(IfCond, UI, &ThenTI, &ElseTI);
+ThenTI->getParent()->setName("omp_if.then");
+ElseTI->getParent()->setName("omp_if.else");

TIFitis wrote:
> kiranchandramohan wrote:
> > TIFitis wrote:
> > > kiranchandramohan wrote:
> > > > There is some recent recommendation to not insert artificial 
> > > > instructions and remove them. The preferred approach is to use 
> > > > `splitBB` function(s) inside the OpenMPIRBuilder. This function works 
> > > > on blocks without terminators. You can consult the `IfCondition` code 
> > > > in `createParallel`.
> > > Thanks a lot for taking the time to review this lengthy patch.
> > > 
> > > This one seems a bit tricky to do. At first glance `createParallel` seems 
> > > to be doing something different where its calling different runtime 
> > > functions based on the `IfCondition` instead of much in the way of 
> > > Control Flow changes.
> > > 
> > > The `unreachable` inst helps out a lot here as it makes it really easy to 
> > > keep trace of the resume point for adding instructions after the 
> > > `BodyGen` codes are generated.
> > > 
> > > I am still looking into finding a way to do this elegantly without having 
> > > to use the `unreachable` instruction, but would it be a major blocker if 
> > > we had to keep it?
> > > 
> > > I have addressed all the other changes you requested.
> > Thanks for explaining the need for the `unreachable`.  I will leave it with 
> > you on whether to make the change.
> > 
> > You can see an instance of a past request for not using temporary 
> > instruction. That patch (if for createTask) addressed the issue one way.
> > https://reviews.llvm.org/D130615#inline-1257711
> > 
> > I gave a quick try and came up with the following code. I don't think it is 
> > very elegant, but it is one way to do it. 
> > ```
> > -  // LLVM utilities like blocks with terminators.
> > -  auto *UI = Builder.CreateUnreachable();
> > +  BasicBlock *ContBB = nullptr;
> >if (IfCond) {
> > -auto *ThenTI =
> > -SplitBlockAndInsertIfThen(IfCond, UI, /* Unreachable */ false);
> > -ThenTI->getParent()->setName("omp_if.then");
> > -Builder.SetInsertPoint(ThenTI);
> > -  } else {
> > -Builder.SetInsertPoint(UI);
> > +BasicBlock *EntryBB = Builder.GetInsertBlock();
> > +ContBB = splitBB(Builder, /*CreateBranch*/ false);
> > +BasicBlock *ThenBB =
> > +BasicBlock::Create(Builder.getContext(), EntryBB->getName() + ".if",
> > +   ContBB->getParent(), ContBB);
> > +ContBB->setName(EntryBB->getName() + ".if.end");
> > +Builder.CreateCondBr(IfCond, ThenBB, ContBB);
> > +Builder.SetInsertPoint(ThenBB);
> > +Builder.CreateBr(ContBB);
> > +Builder.SetInsertPoint(ThenBB->getTerminator());
> >}
> >  
> >ProcessMapOpCB(Builder.saveIP(), Builder.saveIP());
> > @@ -4087,9 +4090,10 @@ OpenMPIRBuilder::InsertPointTy 
> > OpenMPIRBuilder::createTargetData(
> >  emitMapperCall(Builder.saveIP(), beginMapperFunc, srcLocInfo, 
> > MapTypesArg,
> > MapNamesArg, MapperAllocas, DeviceID, 
> > MapTypeFlags.size());
> >  
> > +BasicBlock *DataExitBB = splitBB(Builder, true, "target_data.exit");
> >  BodyGenCB(Builder.saveIP(), Builder.saveIP());
> >  
> > -Builder.SetInsertPoint(UI->getParent());
> > +Builder.SetInsertPoint(DataExitBB, DataExitBB->begin());
> >  // Create call to end the

[PATCH] D145344: [clang-format] Don't annotate left brace of class as FunctionLBrace

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

The upshot of this is better TT_FunctionLBrace/TT_ClassLBrace detection, which 
is great becuase  if a class `{` or function `{` is incorrectly detected we are 
then in unknown territory as to how it might become formatted, since you 
introduced these Annotator tests, I fell we are really helping to lock down the 
other rules.  I would hazard a guess that this actually solves more issues than 
we realize.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145344

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

[PATCH] D145435: Choose style (file) from within code for use in IDEs

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

This is going to need unit tests
A full context diff


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145435

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

[PATCH] D145228: [clangd] Add clangd headers to install targets

[Ivan Murashko via Phabricator via cfe-commits]

ivanmurashko added a comment.

In D145228#4174543 , @kadircet wrote:

> 

I understand concerns about maintenance cost for the change. But I dare to ask 
why you think it is so high? Perhaps there are different expectations from the 
feature. I’m not asking to support any API stability or anything like this. 
IMHO, the maintenance cost for the feature should be on people who would like 
to use it. Clangd can only export headers and libraries as they are with one 
addition to allow main function overriding. Users of the feature should be 
prepared that it might change significantly at any point even within release. 
There is smaller but similar problem with other many other LLVM APIs, that are 
significantly changed especially between releases. I hope this expectation 
should make the maintenance cost for the change smaller and we are happy to 
support them if it is needed.

There are some additional context about the change. We have a separate build 
system (not CMake) for in-house apps. The LVVM is considered as a third-party 
with corresponding libs and headers. The approach allows us to create in-house 
clang tools such as lint and refactoring tools compiling from the same sources 
against multiple clang versions and variants. The approach is supported by LLVM 
modular structure that gives an ability to combine different pieces to create 
powerful customized in-house apps. Use forks of LLVM is one of the obvious way 
to use the feature, but it has high maintenance cost and it makes harder to 
contribute back bug fixes and features. Because we have to pay this cost anyway 
we can support this feature in upstream for everybody and make LLVM even more 
re-usable. If we need to support it in another build system, we can do it as 
well (up to the degree we can in LLVM).

What do you think about it? Have I missed something important?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145228

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

[PATCH] D144651: [Serialization] Place command line defines in the correct file

[John Brawn via Phabricator via cfe-commits]

john.brawn updated this revision to Diff 502977.
john.brawn added a comment.

Ran clang-format.


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

https://reviews.llvm.org/D144651

Files:
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/PCH/macro-cmdline.c
  clang/test/PCH/ms-pch-macro.c

Index: clang/test/PCH/ms-pch-macro.c
===
--- clang/test/PCH/ms-pch-macro.c
+++ clang/test/PCH/ms-pch-macro.c
@@ -36,4 +36,4 @@
 // CHECK-FOO: definition of macro 'FOO' differs between the precompiled header ('1') and the command line ('blah')
 // CHECK-NOFOO: macro 'FOO' was defined in the precompiled header but undef'd on the command line
 
-// expected-warning@1 {{definition of macro 'BAR' does not match definition in precompiled header}}
+// expected-warning@2 {{definition of macro 'BAR' does not match definition in precompiled header}}
Index: clang/test/PCH/macro-cmdline.c
===
--- /dev/null
+++ clang/test/PCH/macro-cmdline.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -emit-pch -o %t1.pch -DMACRO1=1
+// RUN: %clang_cc1 -fsyntax-only %s -include-pch %t1.pch -DMACRO2=1 2>&1 | FileCheck %s
+
+#ifndef HEADER
+#define HEADER
+#else
+#define MACRO1 2
+// CHECK: macro-cmdline.c{{.*}}'MACRO1' macro redefined
+// CHECK: {{.*}}previous definition is here
+#define MACRO2 2
+// CHECK: macro-cmdline.c{{.*}}'MACRO2' macro redefined
+// CHECK: {{.*}}previous definition is here
+#endif
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -,6 +,11 @@
 bool ASTWriter::PreparePathForOutput(SmallVectorImpl &Path) {
   assert(Context && "should have context when outputting path");
 
+  // Leave special file names as they are.
+  StringRef PathStr(Path.data(), Path.size());
+  if (PathStr == "" || PathStr == "")
+return false;
+
   bool Changed =
   cleanPathForOutput(Context->getSourceManager().getFileManager(), Path);
 
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -654,6 +654,10 @@
   SmallVector ExistingMacroNames;
   collectMacroDefinitions(ExistingPPOpts, ExistingMacros, &ExistingMacroNames);
 
+  // Use a line marker to enter the  file, as the defines and
+  // undefines here will have come from the command line.
+  SuggestedPredefines += "# 1 \"\" 1\n";
+
   for (unsigned I = 0, N = ExistingMacroNames.size(); I != N; ++I) {
 // Dig out the macro definition in the existing preprocessor options.
 StringRef MacroName = ExistingMacroNames[I];
@@ -713,6 +717,10 @@
 }
 return true;
   }
+
+  // Leave the  file and return to .
+  SuggestedPredefines += "# 1 \"\" 2\n";
+
   if (Validation == OptionValidateStrictMatches) {
 // If strict matches are requested, don't tolerate any extra defines in
 // the AST file that are missing on the command line.
@@ -1579,8 +1587,13 @@
 auto Buffer = ReadBuffer(SLocEntryCursor, Name);
 if (!Buffer)
   return true;
-SourceMgr.createFileID(std::move(Buffer), FileCharacter, ID,
-   BaseOffset + Offset, IncludeLoc);
+FileID FID = SourceMgr.createFileID(std::move(Buffer), FileCharacter, ID,
+BaseOffset + Offset, IncludeLoc);
+if (Record[3]) {
+  SrcMgr::FileInfo &FileInfo =
+  const_cast(SourceMgr.getSLocEntry(FID).getFile());
+  FileInfo.setHasLineDirectives();
+}
 break;
   }
 
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -687,8 +687,10 @@
 
   const auto &SM = PP->getSourceManager();
   auto DefLoc = MI->getDefinitionLoc();
-  // Also avoid storing predefined macros like __DBL_MIN__.
+  // Also avoid storing macros that aren't defined in any file, i.e. predefined
+  // macros like __DBL_MIN__ and those defined on the command line.
   if (SM.isWrittenInBuiltinFile(DefLoc) ||
+  SM.isWrittenInCommandLineFile(DefLoc) ||
   Name->getName() == "__GCC_HAVE_DWARF2_CFI_ASM")
 return true;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145486: [clang] Fix single-element array initialization in constexpr

[Mariya Podchishchaeva via Phabricator via cfe-commits]

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

https://reviews.llvm.org/D130791 added an improvement that in case array
element has a trivial constructor, it is evaluated once and the result is
re-used for remaining elements. Make sure the constructor is evaluated
for single-elements arrays too.

Fixes https://github.com/llvm/llvm-project/issues/60803


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145486

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constexpr-single-element-array.cpp


Index: clang/test/SemaCXX/constexpr-single-element-array.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constexpr-single-element-array.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+
+/// This test makes sure that single element array in constexpr doesn't produce
+/// spurious errors.
+
+/// expected-no-diagnostics
+struct Sub { int x; };
+
+struct S {
+  constexpr S() { Arr[0] = Sub{}; }
+  Sub Arr[1];
+};
+
+constexpr bool test() {
+  S s;
+  return true;
+}
+
+static_assert(test());
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -10917,7 +10917,7 @@
 for (unsigned I = OldElts; I < N; ++I)
   Value->getArrayInitializedElt(I) = Filler;
 
-  if (HasTrivialConstructor && N == FinalSize) {
+  if (HasTrivialConstructor && N == FinalSize && N != 1) {
 // If we have a trivial constructor, only evaluate it once and copy
 // the result into all the array elements.
 APValue &FirstResult = Value->getArrayInitializedElt(0);


Index: clang/test/SemaCXX/constexpr-single-element-array.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constexpr-single-element-array.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+
+/// This test makes sure that single element array in constexpr doesn't produce
+/// spurious errors.
+
+/// expected-no-diagnostics
+struct Sub { int x; };
+
+struct S {
+  constexpr S() { Arr[0] = Sub{}; }
+  Sub Arr[1];
+};
+
+constexpr bool test() {
+  S s;
+  return true;
+}
+
+static_assert(test());
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -10917,7 +10917,7 @@
 for (unsigned I = OldElts; I < N; ++I)
   Value->getArrayInitializedElt(I) = Filler;
 
-  if (HasTrivialConstructor && N == FinalSize) {
+  if (HasTrivialConstructor && N == FinalSize && N != 1) {
 // If we have a trivial constructor, only evaluate it once and copy
 // the result into all the array elements.
 APValue &FirstResult = Value->getArrayInitializedElt(0);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145228: [clangd] Add clangd headers to install targets

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D145228#4170864 , @sammccall wrote:

> Clangd isn't designed as a collection of libraries.

clangd is not my area of expertise, so I don't intend to step into the middle 
of this with my comment, but: one of the tenants of the LLVM project is that 
it's a modular set of reusable components. clangd being designed such that it 
cannot be used as a library goes against the grain in that regard, so I'm 
surprised to see push-back against attempts to make a more modular, 
component-based design. What am I missing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145228

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

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon added inline comments.



Comment at: clang/test/Driver/flang/flang-omp.f90:1
+! Check that flang -fc1 is invoked when in --driver-mode=flang 
+! and the relevant openmp and openmp offload flags are utilised

awarzynski wrote:
> agozillon wrote:
> > awarzynski wrote:
> > > agozillon wrote:
> > > > awarzynski wrote:
> > > > > agozillon wrote:
> > > > > > agozillon wrote:
> > > > > > > awarzynski wrote:
> > > > > > > > This test looks correct to me, but please note that:
> > > > > > > > 
> > > > > > > > ```
> > > > > > > > ! Check that flang -fc1 is invoked when in --driver-mode=flang 
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > yet (`%clang` instead of `%flang`)
> > > > > > > > 
> > > > > > > > ```
> > > > > > > > ! RUN: %clang --driver-mode=flang -### -fopenmp %s 2>&1 | 
> > > > > > > > FileCheck --check-prefixes=CHECK-OPENMP %s
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > I'm not really sure whether we should be testing Flang-specific 
> > > > > > > > logic in Clang. Having said that, Flang does use `clangDriver` 
> > > > > > > > to implement its driver :) 
> > > > > > > > 
> > > > > > > > You could consider using 
> > > > > > > > https://github.com/llvm/llvm-project/blob/main/flang/test/Driver/frontend-forwarding.f90
> > > > > > > >  instead (or add an OpenMP specific file there).
> > > > > > > > 
> > > > > > > > Not a blocker.
> > > > > > > Yes, I wasn't so sure either as it felt a little weird to test 
> > > > > > > Flang components inside of Clang, but as you said it is a Clang 
> > > > > > > toolchain (that this test is checking) and borrows from the 
> > > > > > > clangDriver! 
> > > > > > > 
> > > > > > > I borrowed this test from other similar tests in the same folder 
> > > > > > > that test other flang specific driver logic in a similar manner, 
> > > > > > > but I am more than happy to add an additional flang specific 
> > > > > > > driver test as you mention! 
> > > > > > On further looking into the frontend-forwarding test, I don't know 
> > > > > > if it is suitable for fopenmp-is-device as it is an fc1 option and 
> > > > > > won't be forwarded from the flang-new frontend down to fc1 at the 
> > > > > > moment! 
> > > > > > 
> > > > > > I think this test will be more suitable when additional flags like 
> > > > > > the fopenmp-targets (or similar flags) that are used in this test 
> > > > > > are added to the Flang driver. As they spawn/propagate the 
> > > > > > openmp-is-device flag. However, perhaps I am incorrect.
> > > > > > On further looking into the frontend-forwarding test, I don't know 
> > > > > > if it is suitable for fopenmp-is-device as it is an fc1 option and 
> > > > > > won't be forwarded from the flang-new frontend down to fc1 at the 
> > > > > > moment!
> > > > > 
> > > > > It should be - that's what the logic in Flang.cpp does, no? And if it 
> > > > > doesn't, is it a deliberate design decision?
> > > > I believe the logic in Flang.h/Flang.cpp just invokes for -fc1 not the 
> > > > Frontend driver, at least that's what it looks like from the 
> > > > ConstructJob task that Clang invokes. 
> > > > 
> > > > It's a deliberate design decision that -fopenmp-is-device is an fc1 
> > > > option, it mimics the way Clang currently does it, which is a cc1 
> > > > option, there is no way to directly specify it to clang without -cc1 I 
> > > > think. The hope is to keep the Flang OpenMP flags as similar to Clang's 
> > > > as possible for the time being I believe.
> > > > I believe the logic in Flang.h/Flang.cpp just invokes for -fc1 not the 
> > > > Frontend driver.
> > > 
> > > `flang-new -fc1` **is** the frontend driver ;-) So:
> > > * you've added the  logic to forward `-fopenmp-is-device` that should 
> > > work for both `flang-new` and `clang --driver-mode=flang`, but
> > > * you're only testing one of these.
> > > 
> > > There should have tests for both. In particular for `flang-new` given 
> > > that this patch adds new functionality to LLVM Flang. If that doesn't 
> > > work, let's try to figure it out together.
> > > 
> > > > The hope is to keep the Flang OpenMP flags as similar to Clang's as 
> > > > possible for the time being I believe.
> > > 
> > > +1 We should try as much as we can to keep Clang's and Flang's behavior 
> > > consistent :) (which you do 👍🏻 )
> > Ah I didn't realise that, thank you very much for clarifying that for me! 
> > :) I thought it was bypassing it with -fc1, and yjsy flang-new without it 
> > was the frontend! I still have a lot to learn, so I apologies for the 
> > misunderstanding! 
> > 
> > In this case do you mean test with "flang-new -fc1 -fopenmp-is-device" or 
> > "flang-new -fopenmp-is-device", as the latter will not be accepted by 
> > flang-new in the current patch but I do test the -fc1 variation in the 
> > omp-is-device.f90 test which is more about testing the attribute admittedly 
> > as opposed to the driver command. 
> > 
> > I don't know if testing it in the frontend-

[PATCH] D145486: [clang] Fix single-element array initialization in constexpr

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 502992.
Fznamznon added a comment.

Check FinalSize instead of N since it is probably more obvious


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145486

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constexpr-single-element-array.cpp


Index: clang/test/SemaCXX/constexpr-single-element-array.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constexpr-single-element-array.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+
+/// This test makes sure that single element array in constexpr doesn't produce
+/// spurious errors.
+
+/// expected-no-diagnostics
+struct Sub { int x; };
+
+struct S {
+  constexpr S() { Arr[0] = Sub{}; }
+  Sub Arr[1];
+};
+
+constexpr bool test() {
+  S s;
+  return true;
+}
+
+static_assert(test());
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -10917,7 +10917,7 @@
 for (unsigned I = OldElts; I < N; ++I)
   Value->getArrayInitializedElt(I) = Filler;
 
-  if (HasTrivialConstructor && N == FinalSize) {
+  if (HasTrivialConstructor && N == FinalSize && FinalSize != 1) {
 // If we have a trivial constructor, only evaluate it once and copy
 // the result into all the array elements.
 APValue &FirstResult = Value->getArrayInitializedElt(0);


Index: clang/test/SemaCXX/constexpr-single-element-array.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constexpr-single-element-array.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+
+/// This test makes sure that single element array in constexpr doesn't produce
+/// spurious errors.
+
+/// expected-no-diagnostics
+struct Sub { int x; };
+
+struct S {
+  constexpr S() { Arr[0] = Sub{}; }
+  Sub Arr[1];
+};
+
+constexpr bool test() {
+  S s;
+  return true;
+}
+
+static_assert(test());
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -10917,7 +10917,7 @@
 for (unsigned I = OldElts; I < N; ++I)
   Value->getArrayInitializedElt(I) = Filler;
 
-  if (HasTrivialConstructor && N == FinalSize) {
+  if (HasTrivialConstructor && N == FinalSize && FinalSize != 1) {
 // If we have a trivial constructor, only evaluate it once and copy
 // the result into all the array elements.
 APValue &FirstResult = Value->getArrayInitializedElt(0);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145228: [clangd] Add clangd headers to install targets

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

@aaron.ballman

If someone wants to work on producing a more modular, component-based design, 
that's probably a good idea (but tricky to get right). Probably a good place to 
start is working out what pieces can be moved into separate libraries with e.g. 
a coherent layering story, thinking about how they might be reused in other 
tools etc.

Exposing all clangd's headers as-is is not that. I don't think the idea that 
llvm-project is modular is at odds with the idea that leaf subprojects exist, 
though clangd is big enough that it may make sense to split up. And private 
headers (those that live next to source files in the tree) are not installed 
(with the recent exception of `clang-tidy`, which IMO should have been 
restructured to publish in this way).

FWIW, I do think clang is a fair example of a project where the default of 
"everything is a public library" has made the library design somewhat 
incoherent and hard to evolve. OTOH it's enabled tons of useful stuff, so...

@ivanmurashko

Yes, my experience is that in practice people object to e.g. removing things 
without replacement, even in an "unstable" API.
Heck, *I* object to that, because it's often disruptive, and in practice people 
do put effort into making transitions easier.

I think this is a bit abstract though. Concretely, what API do you need here? 
e.g. which headers do you want to include, to what end?

My impression so far is that you actually don't need an API, but rather to link 
some extra libraries into an otherwise-unmodified clangd binary, and that being 
able to call `clangdMain()` from an external source file would make this easier 
in your build system. That seems like a problem worth solving, but installing 
all clangd's private headers into /usr/lib doesn't seem like a particularly 
direct solution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145228

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

[PATCH] D145228: [clangd] Add clangd headers to install targets

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D145228#4174829 , @sammccall wrote:

> @aaron.ballman
>
> If someone wants to work on producing a more modular, component-based design, 
> that's probably a good idea (but tricky to get right). Probably a good place 
> to start is working out what pieces can be moved into separate libraries with 
> e.g. a coherent layering story, thinking about how they might be reused in 
> other tools etc.
>
> Exposing all clangd's headers as-is is not that. I don't think the idea that 
> llvm-project is modular is at odds with the idea that leaf subprojects exist, 
> though clangd is big enough that it may make sense to split up. And private 
> headers (those that live next to source files in the tree) are not installed 
> (with the recent exception of `clang-tidy`, which IMO should have been 
> restructured to publish in this way).

Ah, okay, thank you for the clarification! I agree we have to go about 
splitting the project up into libraries very carefully and this approach isn't 
appropriate. But I was getting the impression there was push-back against the 
idea of making clangd into a series of libraries in general and that worried me.

> FWIW, I do think clang is a fair example of a project where the default of 
> "everything is a public library" has made the library design somewhat 
> incoherent and hard to evolve. OTOH it's enabled tons of useful stuff, so...

IMO, we've not really had too many issues evolving Clang's libraries, but I do 
think there's room for improvement (it's hard to figure out how to split up 
Sema, for example). As you say, it's not an easy problem to solve but it 
enables a lot of really useful stuff we wouldn't otherwise be able to achieve 
as easily.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145228

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

[clang] 533997b - Fix DISABLE-NOT: cc1 check in debug-info-codeview-buildinfo.c test

[Matt Devereau via cfe-commits]

Author: Matt Devereau
Date: 2023-03-07T13:22:18Z
New Revision: 533997b026bab9994209d07ab6297d1482289f2d

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

LOG: Fix DISABLE-NOT: cc1 check in debug-info-codeview-buildinfo.c test

This check is checking for the cc1 flag but this test has been seen to fail
when FILEPATHVAL has contained cc1 in generated SHAs

Added: 


Modified: 
clang/test/CodeGen/debug-info-codeview-buildinfo.c

Removed: 




diff  --git a/clang/test/CodeGen/debug-info-codeview-buildinfo.c 
b/clang/test/CodeGen/debug-info-codeview-buildinfo.c
index 4096fac5f7437..4fc55af10a6e2 100644
--- a/clang/test/CodeGen/debug-info-codeview-buildinfo.c
+++ b/clang/test/CodeGen/debug-info-codeview-buildinfo.c
@@ -33,7 +33,7 @@ int main(void) { return 42; }
 // RELATIVE: 0x{{.+}} | LF_BUILDINFO [size = {{.+}}]
 // RELATIVE:  0x{{.+}}: `.`
 
-// DISABLE-NOT: cc1
+// DISABLE-NOT: "-cc1"
 // DISABLE: 0x{{.+}} | LF_BUILDINFO [size = {{.+}}]
 // DISABLE-NEXT:  0x{{.+}}: `{{.*}}`
 // DISABLE-NEXT:  : ``



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

[PATCH] D145491: [clang] Replace Member Expressions During Instantiation If Necessary

[Liming Liu via Phabricator via cfe-commits]

lime created this revision.
lime added reviewers: aaron.ballman, cor3ntin, erichkeane, clang-language-wg.
lime added projects: All, clang, clang-language-wg.
lime requested review of this revision.
Herald added a subscriber: cfe-commits.

This patch replace member accesses to declaration references during template
instantiation if the context is the unevaluated context and the class does not
contain the declaration.

The replacement fixes the issue #58674. Unlike previous fixes such as D143840 
,
it checks the membership during instantiation rather than right after parsing,
so the check is more accurate and efficient.

This patch also includes cases that previous fixes had once failed on.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145491

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/TreeTransform.h
  clang/test/CodeGenCXX/decl-ref-inheritance.cpp
  clang/test/SemaCXX/decltype.cpp

Index: clang/test/SemaCXX/decltype.cpp
===
--- clang/test/SemaCXX/decltype.cpp
+++ clang/test/SemaCXX/decltype.cpp
@@ -101,6 +101,44 @@
   template void foo(decltype(T(LP1{ .p1 = g1, .p1.x[1] = 'x' }))) {}
 }
 
+namespace GH58674 {
+  struct Foo {
+float value_;
+struct nested {
+  float value_;
+};
+  };
+
+  template 
+  struct TemplateFoo {
+float value_;
+  };
+
+  float bar;
+
+  template 
+  struct Animal{};
+
+  template 
+  class Cat : Animal {
+using okay = decltype(Foo::value_);
+using also_okay = decltype(bar);
+using okay2 = decltype(Foo::nested::value_);
+using okay3 = decltype(TemplateFoo::value_);
+  public:
+void meow() {
+  using okay = decltype(Foo::value_);
+  using also_okay = decltype(bar);
+  using okay2 = decltype(Foo::nested::value_);
+  using okay3 = decltype(TemplateFoo::value_);
+}
+  };
+
+  void baz() {
+  Cat{}.meow();
+  }
+}
+
 template
 class conditional {
 };
Index: clang/test/CodeGenCXX/decl-ref-inheritance.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/decl-ref-inheritance.cpp
@@ -0,0 +1,60 @@
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux -emit-llvm %s -o - | FileCheck \
+// RUN: -check-prefix=CHECK-1 %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux -emit-llvm %s -o - | FileCheck \
+// RUN: -check-prefix=CHECK-2 %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux -emit-llvm %s -o - | FileCheck \
+// RUN: -check-prefix=CHECK-3 %s
+
+// CHECK-1: [[FOO:%.+]] = type { float }
+struct foo {
+  float val;
+};
+
+template  struct bar : T {
+};
+
+struct baz : bar {
+  // CHECK-1: define{{.*}} float @_ZN3baz3getEv
+  // CHECK-1: {{%.+}} = getelementptr inbounds [[FOO]], ptr {{%.+}}, i32 0, i32 0
+  float get() {
+return val;
+  }
+};
+
+int qux() {
+  auto f = baz{};
+  return f.get();
+}
+
+// CHECK-2: [[F:%.+]] = type { ptr }
+struct f {
+  void *g;
+};
+
+template  struct k : j {
+  // CHECK-2: define{{.*}} void @_ZN1kI1fE1lEv
+  // CHECK-2: {{%.+}} = getelementptr inbounds [[F]], ptr {{%.+}}, i32 0, i32 0
+  virtual void l(){ (void)f::g; }
+};
+
+k q;
+
+// CHECK-3: [[BASE:%.+]] = type { i32 }
+class Base {
+protected:
+  int member;
+};
+
+template 
+struct Subclass : public Parent {
+  // CHECK-3: define{{.*}} i32 @_ZN8SubclassI4BaseE4funcEv
+  // CHECK-3: {{%.+}} = getelementptr inbounds [[BASE]], ptr {{%.+}}, i32 0, i32 0
+  int func() { return Base::member; }
+};
+
+using Impl = Subclass;
+
+int use() {
+  Impl i;
+  return i.func();
+}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -2803,6 +2803,21 @@
 R.addDecl(FoundDecl);
 R.resolveKind();
 
+if (getSema().isUnevaluatedContext() && Base->isImplicitCXXThis() &&
+isa(Member)) {
+  if (auto ThisClass = cast(Base)
+   ->getType()
+   ->getPointeeType()
+   ->getAsCXXRecordDecl()) {
+auto Class = cast(Member->getDeclContext());
+// In unevaluated contexts, an expression supposed to be a member access
+// might reference a member in an unrelated class.
+if (!ThisClass->Equals(Class) && !ThisClass->isDerivedFrom(Class))
+  return getSema().BuildDeclRefExpr(Member, Member->getType(),
+VK_LValue, Member->getLocation());
+  }
+}
+
 return getSema().BuildMemberReferenceExpr(Base, BaseType, OpLoc, isArrow,
   SS, TemplateKWLoc,
   FirstQualifierInScope,
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -194,6 +194,9 @@
 - Fix crash when evaluating conste

[clang-tools-extra] cc92959 - [libclang] Add API to override preamble storage path

[Aaron Ballman via cfe-commits]

Author: Igor Kushnir
Date: 2023-03-07T08:25:38-05:00
New Revision: cc929590ad305f0d068709c7c7999f5fc6118dc9

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

LOG: [libclang] Add API to override preamble storage path

TempPCHFile::create() calls llvm::sys::fs::createTemporaryFile() to
create a file named preamble-*.pch in a system temporary directory. This
commit allows overriding the directory where these often many and large
preamble-*.pch files are stored.

The referenced bug report requests the ability to override the temporary
directory path used by libclang. However, overriding the return value of
llvm::sys::path::system_temp_directory() was rejected during code review
as improper and because it would negatively affect multithreading
performance. Finding all places where libclang uses the temporary
directory is very difficult. Therefore this commit is limited to
override libclang's single known use of the temporary directory.

This commit allows to override the preamble storage path only during
CXIndex construction to avoid multithreading issues and ensure that all
preambles are stored in the same directory. For the same multithreading
and consistency reasons, this commit deprecates
clang_CXIndex_setGlobalOptions() and
clang_CXIndex_setInvocationEmissionPathOption() in favor of specifying
these options during CXIndex construction.

Adding a new CXIndex constructor function each time a new initialization
argument is needed leads to either a large number of function parameters
unneeded by most libclang users or to an exponential number of overloads
that support different usage requirements. Therefore this commit
introduces a new extensible struct CXIndexOptions and a general function
clang_createIndexWithOptions().

A libclang user passes a desired preamble storage path to
clang_createIndexWithOptions(), which stores it in
CIndexer::PreambleStoragePath. Whenever
clang_parseTranslationUnit_Impl() is called, it passes
CIndexer::PreambleStoragePath to ASTUnit::LoadFromCommandLine(), which
stores this argument in ASTUnit::PreambleStoragePath. Whenever
ASTUnit::getMainBufferWithPrecompiledPreamble() is called, it passes
ASTUnit::PreambleStoragePath to PrecompiledPreamble::Build().
PrecompiledPreamble::Build() forwards the corresponding StoragePath
argument to TempPCHFile::create(). If StoragePath is not empty,
TempPCHFile::create() stores the preamble-*.pch file in the directory at
the specified path rather than in the system temporary directory.

The analysis below proves that this passing around of the
PreambleStoragePath string is sufficient to guarantee that the libclang
user override is used in TempPCHFile::create(). The analysis ignores API
uses in test code.

TempPCHFile::create() is called only in PrecompiledPreamble::Build().
PrecompiledPreamble::Build() is called only in two places: one in
clangd, which is not used by libclang, and one in
ASTUnit::getMainBufferWithPrecompiledPreamble().
ASTUnit::getMainBufferWithPrecompiledPreamble() is called in 3 places:

ASTUnit::LoadFromCompilerInvocation() [analyzed below].
ASTUnit::Reparse(), which in turn is called only from
clang_reparseTranslationUnit_Impl(), which in turn is called only from
clang_reparseTranslationUnit(). clang_reparseTranslationUnit() is never
called in LLVM code, but is part of public libclang API. This function's
documentation requires its translation unit argument to have been built
with clang_createTranslationUnitFromSourceFile().
clang_createTranslationUnitFromSourceFile() delegates its work to
clang_parseTranslationUnit(), which delegates to
clang_parseTranslationUnit2(), which delegates to
clang_parseTranslationUnit2FullArgv(), which delegates to
clang_parseTranslationUnit_Impl(), which passes
CIndexer::PreambleStoragePath to the ASTUnit it creates.

ASTUnit::CodeComplete() passes AllowRebuild = false to
ASTUnit::getMainBufferWithPrecompiledPreamble(), which makes it return
nullptr before calling PrecompiledPreamble::Build().

Both ASTUnit::LoadFromCompilerInvocation() overloads (one of which
delegates its work to another) call
ASTUnit::getMainBufferWithPrecompiledPreamble() only if their argument
PrecompilePreambleAfterNParses > 0. LoadFromCompilerInvocation() is
called in:

ASTBuilderAction::runInvocation() keeps the default parameter value
of PrecompilePreambleAfterNParses = 0, meaning that the preamble file is
never created from here.

ASTUnit::LoadFromCommandLine().
ASTUnit::LoadFromCommandLine() is called in two places:

CrossTranslationUnitContext::ASTLoader::loadFromSource() keeps the
default parameter value of PrecompilePreambleAfterNParses = 0, meaning
that the preamble file is never created from here.

clang_parseTranslationUnit_Impl(), which passes
CIndexer::PreambleStoragePath to the ASTUnit it creates.

Therefore, the overridden preamble storage path is always used in
TempPCHFi

[PATCH] D143418: [libclang] Add API to override preamble storage path

[Aaron Ballman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcc929590ad30: [libclang] Add API to override preamble 
storage path (authored by vedgy, committed by aaron.ballman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143418

Files:
  clang-tools-extra/clangd/Preamble.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang-c/Index.h
  clang/include/clang/Frontend/ASTUnit.h
  clang/include/clang/Frontend/PrecompiledPreamble.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/PrecompiledPreamble.cpp
  clang/tools/c-index-test/c-index-test.c
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/CIndexer.h
  clang/tools/libclang/libclang.map
  clang/unittests/Frontend/ASTUnitTest.cpp
  clang/unittests/libclang/LibclangTest.cpp
  clang/unittests/libclang/TestUtils.h

Index: clang/unittests/libclang/TestUtils.h
===
--- clang/unittests/libclang/TestUtils.h
+++ clang/unittests/libclang/TestUtils.h
@@ -22,11 +22,11 @@
 #include 
 
 class LibclangParseTest : public ::testing::Test {
-  // std::greater<> to remove files before their parent dirs in TearDown().
-  std::set> Files;
   typedef std::unique_ptr fixed_addr_string;
   std::map UnsavedFileContents;
 public:
+  // std::greater<> to remove files before their parent dirs in TearDown().
+  std::set> FilesAndDirsToRemove;
   std::string TestDir;
   bool RemoveTestDirRecursivelyDuringTeardown = false;
   CXIndex Index;
@@ -40,7 +40,7 @@
 TestDir = std::string(Dir.str());
 TUFlags = CXTranslationUnit_DetailedPreprocessingRecord |
   clang_defaultEditingTranslationUnitOptions();
-Index = clang_createIndex(0, 0);
+CreateIndex();
 ClangTU = nullptr;
   }
   void TearDown() override {
@@ -48,7 +48,7 @@
 clang_disposeIndex(Index);
 
 namespace fs = llvm::sys::fs;
-for (const std::string &Path : Files)
+for (const std::string &Path : FilesAndDirsToRemove)
   EXPECT_FALSE(fs::remove(Path, /*IgnoreNonExisting=*/false));
 if (RemoveTestDirRecursivelyDuringTeardown)
   EXPECT_FALSE(fs::remove_directories(TestDir, /*IgnoreErrors=*/false));
@@ -63,7 +63,7 @@
FileI != FileEnd; ++FileI) {
 ASSERT_NE(*FileI, ".");
 path::append(Path, *FileI);
-Files.emplace(Path.str());
+FilesAndDirsToRemove.emplace(Path.str());
   }
   Filename = std::string(Path.str());
 }
@@ -101,6 +101,9 @@
 return string;
   };
 
+protected:
+  virtual void CreateIndex() { Index = clang_createIndex(0, 0); }
+
 private:
   template
   static CXChildVisitResult TraverseStateless(CXCursor cx, CXCursor parent,
Index: clang/unittests/libclang/LibclangTest.cpp
===
--- clang/unittests/libclang/LibclangTest.cpp
+++ clang/unittests/libclang/LibclangTest.cpp
@@ -6,6 +6,7 @@
 //
 //===--===//
 
+#include "TestUtils.h"
 #include "clang-c/Index.h"
 #include "clang-c/Rewrite.h"
 #include "llvm/ADT/StringRef.h"
@@ -14,7 +15,7 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include "gtest/gtest.h"
-#include "TestUtils.h"
+#include 
 #include 
 #include 
 #include 
@@ -355,6 +356,168 @@
   clang_ModuleMapDescriptor_dispose(MMD);
 }
 
+TEST_F(LibclangParseTest, GlobalOptions) {
+  EXPECT_EQ(clang_CXIndex_getGlobalOptions(Index), CXGlobalOpt_None);
+}
+
+class LibclangIndexOptionsTest : public LibclangParseTest {
+  virtual void AdjustOptions(CXIndexOptions &Opts) {}
+
+protected:
+  void CreateIndex() override {
+CXIndexOptions Opts;
+memset(&Opts, 0, sizeof(Opts));
+Opts.Size = sizeof(CXIndexOptions);
+AdjustOptions(Opts);
+Index = clang_createIndexWithOptions(&Opts);
+ASSERT_TRUE(Index);
+  }
+};
+
+TEST_F(LibclangIndexOptionsTest, GlobalOptions) {
+  EXPECT_EQ(clang_CXIndex_getGlobalOptions(Index), CXGlobalOpt_None);
+}
+
+class LibclangIndexingEnabledIndexOptionsTest
+: public LibclangIndexOptionsTest {
+  void AdjustOptions(CXIndexOptions &Opts) override {
+Opts.ThreadBackgroundPriorityForIndexing = CXChoice_Enabled;
+  }
+};
+
+TEST_F(LibclangIndexingEnabledIndexOptionsTest, GlobalOptions) {
+  EXPECT_EQ(clang_CXIndex_getGlobalOptions(Index),
+CXGlobalOpt_ThreadBackgroundPriorityForIndexing);
+}
+
+class LibclangIndexingDisabledEditingEnabledIndexOptionsTest
+: public LibclangIndexOptionsTest {
+  void AdjustOptions(CXIndexOptions &Opts) override {
+Opts.ThreadBackgroundPriorityForIndexing = CXChoice_Disabled;
+Opts.ThreadBackgroundPriorityForEditing = CXChoice_Enabled;
+  }
+};
+
+TEST_F(LibclangIndexingDisabledEditingEnabledIndexOptionsTest, GlobalOptions) {
+  EXPECT_EQ(clang_CXIndex_getGlobalOptions(Index),
+CXGlobalOpt_ThreadBackgroundPriorityForEditing);
+}
+
+class LibclangBothEnabledInde

[PATCH] D143418: [libclang] Add API to override preamble storage path

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D143418#4174521 , @vedgy wrote:

> In D143418#4172587 , @aaron.ballman 
> wrote:
>
>> Thank you, this LGTM! I have to head out shortly, so I'll land this on your 
>> behalf tomorrow when I have the time to babysit the postcommit build farm. 
>> However, if you'd like to request commit access for yourself, I think that'd 
>> be reasonable as well: 
>> https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access Let me 
>> know which route you'd prefer going.
>
> https://llvm.org/docs/Phabricator.html#committing-a-change says:
>
>> Using the Arcanist tool can simplify the process of committing reviewed code 
>> as it will retrieve reviewers, the Differential Revision, etc from the 
>> review and place it in the commit message. You may also commit an accepted 
>> change directly using git push, per the section in the getting started guide.
>
> But how to use the Arcanist tool to push reviewed changes is not elaborated. 
> As far as I can tell from the Arcanist documentation, if I have the commit in 
> my local //main// branch which is currently checked out, I simply need to run 
> `arc land` without arguments. Hopefully the Git pre-push hook 
>  I have set up 
> will run in this case.
>
> I have no idea how to babysit the postcommit build farm, and so wouldn't 
> commit when you don't have time anyway. I plan to make only one more change 
> to libclang in the foreseeable future, so not sure learning to handle 
> postcommit issues is justified. I'll leave this to your discretion as I have 
> no idea how difficult and time-consuming this work is, compared to learning 
> how to do it.

Thanks for the explanation, I'm happy to land on your behalf (esp if you don't 
plan to make many more changes in the future). I landed the changes in 
cc929590ad305f0d068709c7c7999f5fc6118dc9 
. I'm not 
of much help with arcanist as I've never actually used it, but I think you're 
correct about only needing to do `arc land`. In terms of babysitting the build 
farm, there's not too much to it -- most bots are configured to send an email 
to the folks on the blamelist when an issue is found. It's mostly a matter of 
being around to check your email, but you can manually watch from 
https://lab.llvm.org/buildbot/#/builders if you enjoy that sort of thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143418

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

[PATCH] D136919: [X86][RFC] Change mangle name of __bf16 from u6__bf16 to DF16b

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.

In D136919#4174513 , @stuij wrote:

> FWIW, at Arm we decided to keep the old name mangling to minimise friction 
> with existing code/libraries, but allow more operations with this same 
> name-mangling. We also discussed with Red Hat and they were ok with this.

Thanks for the information! I'll see if we can do the same for X86.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136919

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

[PATCH] D127812: [AArch64] FMV support and necessary target features dependencies.

[Nikita Popov via Phabricator via cfe-commits]

nikic added inline comments.



Comment at: compiler-rt/lib/builtins/cpu_model.c:1338
+  hwcap = getauxval(AT_HWCAP);
+  hwcap2 = getauxval(AT_HWCAP2);
+#endif // defined(__FreeBSD__)

This breaks the build with glibc 2.17.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127812

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

[PATCH] D145486: [clang] Fix single-element array initialization in constexpr

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, thank you for this! Please be sure to file an issue to backport this to 
the 16.x branch 
(https://llvm.org/docs/GitHub.html#backporting-fixes-to-the-release-branches).




Comment at: clang/test/SemaCXX/constexpr-single-element-array.cpp:3-6
+/// This test makes sure that single element array in constexpr doesn't produce
+/// spurious errors.
+
+/// expected-no-diagnostics

Minor rewording and removing the extra `/` in the comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145486

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

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon updated this revision to Diff 503008.
agozillon added a comment.

- [Flang][ToolChain][OpenMP][Driver] Reduce changes to only add -fc1 support


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144864

Files:
  clang/include/clang/Driver/Options.td
  flang/include/flang/Frontend/LangOptions.def
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/test/Driver/driver-help.f90
  flang/test/Lower/OpenMP/omp-is-device.f90
  flang/tools/bbc/bbc.cpp
  mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
  mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

Index: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
===
--- mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -1417,6 +1417,26 @@
   return success();
 }
 
+//===--===//
+// OpenMPDialect helper functions
+//===--===//
+
+// Set the omp.is_device attribute on the module with the specified boolean
+void OpenMPDialect::setIsDevice(Operation* module, bool isDevice) {
+  module->setAttr(
+  mlir::StringAttr::get(module->getContext(), llvm::Twine{"omp.is_device"}),
+  mlir::BoolAttr::get(module->getContext(), isDevice));
+}
+
+// Return the value of the omp.is_device attribute stored in the module if it
+// exists, otherwise return false by default
+bool OpenMPDialect::getIsDevice(Operation* module) {
+  if (Attribute isDevice = module->getAttr("omp.is_device"))
+if (isDevice.isa())
+  return isDevice.dyn_cast().getValue();
+  return false;
+}
+
 #define GET_ATTRDEF_CLASSES
 #include "mlir/Dialect/OpenMP/OpenMPOpsAttributes.cpp.inc"
 
Index: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
===
--- mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -28,6 +28,15 @@
   let cppNamespace = "::mlir::omp";
   let dependentDialects = ["::mlir::LLVM::LLVMDialect"];
   let useDefaultAttributePrinterParser = 1;
+
+  let extraClassDeclaration = [{
+// Set the omp.is_device attribute on the module with the specified boolean
+static void setIsDevice(Operation* module, bool isDevice);
+
+// Return the value of the omp.is_device attribute stored in the module if it
+// exists, otherwise return false by default
+static bool getIsDevice(Operation* module);
+  }];
 }
 
 // OmpCommon requires definition of OpenACC_Dialect.
Index: flang/tools/bbc/bbc.cpp
===
--- flang/tools/bbc/bbc.cpp
+++ flang/tools/bbc/bbc.cpp
@@ -38,6 +38,7 @@
 #include "flang/Semantics/semantics.h"
 #include "flang/Semantics/unparse-with-symbols.h"
 #include "flang/Version.inc"
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
 #include "mlir/IR/AsmState.h"
 #include "mlir/IR/BuiltinOps.h"
 #include "mlir/IR/MLIRContext.h"
@@ -122,6 +123,11 @@
 llvm::cl::desc("enable openmp"),
 llvm::cl::init(false));
 
+static llvm::cl::opt
+enableOpenMPDevice("fopenmp-is-device",
+   llvm::cl::desc("enable openmp device compilation"),
+   llvm::cl::init(false));
+
 static llvm::cl::opt enableOpenACC("fopenacc",
  llvm::cl::desc("enable openacc"),
  llvm::cl::init(false));
@@ -237,6 +243,8 @@
   kindMap, loweringOptions, {});
   burnside.lower(parseTree, semanticsContext);
   mlir::ModuleOp mlirModule = burnside.getModule();
+  if (enableOpenMP)
+mlir::omp::OpenMPDialect::setIsDevice(mlirModule, enableOpenMPDevice);
   std::error_code ec;
   std::string outputName = outputFilename;
   if (!outputName.size())
Index: flang/test/Lower/OpenMP/omp-is-device.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/omp-is-device.f90
@@ -0,0 +1,19 @@
+!RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=DEVICE
+!RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s --check-prefix=HOST
+!RUN: %flang_fc1 -emit-fir -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=DEVICE-FLAG-ONLY
+!RUN: bbc -fopenmp -fopenmp-is-device -emit-fir -o - %s | FileCheck %s --check-prefix=BBC-DEVICE
+!RUN: bbc -fopenmp -emit-fir -o - %s | FileCheck %s --check-prefix=BBC-HOST
+!RUN: bbc -fopenmp-is-device -emit-fir -o - %s | FileCheck %s --check-prefix=BBC-DEVICE-FLAG-ONLY
+
+!DEVICE: module attributes {{{.*}}, omp.is_device = true{{.*}}}
+!HOST: module attributes {{{.*}}, omp.is_device = false{{.*}}}
+!DEVICE-FLAG-ONLY: module attributes {{{.*}}"
+!DEVICE-FLAG-ONLY-NOT: , omp.is_device =

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon added a comment.

The recent update removed the added offload related code from Flang.h/.cpp and 
removed the related test.

Would it be possible to have an extra sign-off as requested by @awarzynski (or 
more review points to correct/discuss if we aren't happy with the 
state/direction of the patch) from either @kiranchandramohan or @jdoerfert my 
apologies for the pings!

And then provided @awarzynski is happy with the current state of the patch 
after this recent commit and no further comments I can commit and close the 
patch :)

Thank you very much for all the excellent comments in this review, and the 
attention given to it, I appreciate it greatly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144864

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

[PATCH] D145093: Add map info for dereference pointer.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7489-7493
+  if (UO && UO->getOpcode() == UO_Deref)
+if (isa(Last->getAssociatedExpression()) ||
+isa(Last->getAssociatedExpression()) ||
+isa(Last->getAssociatedExpression()))
+  IsVarDerefAssoWithArray = true;

jyu2 wrote:
> ABataev wrote:
> > jyu2 wrote:
> > > ABataev wrote:
> > > > What if we have something like:
> > > > ```
> > > > typedef int (T)[3];
> > > > 
> > > > T** p;
> > > > map(**p)
> > > > ```
> > > > ? Shall it work? I.e. mapping of the whole array by dereferening the 
> > > > pointer to the array.
> > > No, it is not work currently.  What about *(*(p+a)+b)...
> > My question - shall it work? Mapping  (**a)[:3] should result in the same 
> > in the same code for **a if a is
> > ```
> > typedef int(T)[3];
> > T** a;
> > ```
> Oh, that should work.  I add additional test for that.
> 
> ```
> typedef int(T)[3];
> void bar()
> {
> 
> T** a;
> int b[2][3];
> int (*p)[3] = b;
> a =  &p;
> printf("%p %p p %p\n", &a, b, *p);
> for (int i = 0; i< 3; i++) {
>   (**a)[1] = i;
> }
> #pragma omp target map((**a)[:3])
> {
>  (**a)[1]=5;
>   // CHECK: 5
>   printf("a = %d\n", (**a)[1]);
> }
> }
> ```
> 
No, I meant different thing:
```
T **a;
.. map (**a)
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145093

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

[PATCH] D141714: Fix ast print of variables with attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: erichkeane.
aaron.ballman added inline comments.



Comment at: clang/lib/AST/DeclPrinter.cpp:52-58
+enum AttrPrintLoc {
+  SIDE_NONE = 0,
+  SIDE_LEFT = 1,
+  SIDE_MIDDLE = 2,
+  SIDE_RIGHT = 4,
+  SIDE_ANY = SIDE_LEFT | SIDE_MIDDLE | SIDE_RIGHT,
+};





Comment at: clang/lib/AST/DeclPrinter.cpp:60-61
+
+void prettyPrintAttributes(Decl *D, raw_ostream &out,
+   AttrPrintLoc loc = SIDE_ANY);
+





Comment at: clang/lib/AST/DeclPrinter.cpp:252-253
 
-void DeclPrinter::prettyPrintAttributes(Decl *D) {
+void DeclPrinter::prettyPrintAttributes(Decl *D, llvm::raw_ostream &out,
+AttrPrintLoc loc) {
   if (Policy.PolishForDeclaration)





Comment at: clang/lib/AST/DeclPrinter.cpp:267
+  // 3- should be print on the right side of a decl.
+  AttrPrintLoc attrloc = SIDE_MIDDLE; // We default to middle.
+  attr::Kind kind = A->getKind();

FWIW, I'm more used to seeing `__declspec` and `__attribute__` leading the 
declaration specifiers instead of trailing them. Also, "middle" raises 
questions for me as to where the attribute will go for a function. Given:
```
__attribute__((foo)) void bar(void);
```
what happens? All the test coverage that's changed currently is for variables, 
not for functions, so I can't really tell.

Also, this should be tested with keyword attributes like `alignas` and 
`__global` because those have very specific parse orders.



Comment at: clang/lib/AST/DeclPrinter.cpp:273-274
+  if (A->isCXX11Attribute()) {
+// C++11 onwards attributes can not be placed on middle. Output on
+// right to mimic old clang behaviour.
+attrloc = SIDE_RIGHT;

This comment isn't quite accurate -- it's more that `[[]]` attributes have very 
specific rules about what they appertain to based on the syntactic location of 
the `[[]]`. e.g.,
```
[[foo]] int a, b; // foo applies to both a and b
int [[foo]] a, b; // foo applies to the type of int, which forms the type for 
both a and b
int a [[foo]], b; // foo applies to just a
```
Also, the same logic applies to `[[]]` in C as well as C++, so I think you 
meant to use `A->isStandardAttributeSyntax()`.



Comment at: clang/test/Analysis/blocks.mm:73-74
 
+// FIXME: C++ issues a ignore warning on this __attribute__ output.
+
 // CHECK-LABEL:void testBlockWithCaptureByReference()

Can you explain a bit more about what warning you're seeing and under what 
condition?



Comment at: clang/test/Sema/attr-print.c:4
-// CHECK: int x __attribute__((aligned(4)));
-int x __attribute__((aligned(4)));
 

Why did you change where the attribute is written in this test?



Comment at: clang/test/Sema/attr-print.c:8-11
-__declspec(align(4)) int y;
+// CHECK: int __declspec(align(4)) y;
+int __declspec(align(4)) y;
 
-// CHECK: short arr[3] __attribute__((aligned));
-short arr[3] __attribute__((aligned));

Same questions here.



Comment at: clang/test/SemaCXX/attr-print.cpp:3-4
 
-// CHECK: int x __attribute__((aligned(4)));
-int x __attribute__((aligned(4)));
 

Same questions here as above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141714

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

[PATCH] D143496: [clangd] Add support for missing includes analysis.

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo updated this revision to Diff 503012.
VitaNuo marked 10 inline comments as done.
VitaNuo added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143496

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
  clang-tools-extra/clangd/unittests/PreambleTests.cpp
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
  clang-tools-extra/include-cleaner/lib/Analysis.cpp

Index: clang-tools-extra/include-cleaner/lib/Analysis.cpp
===
--- clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -49,8 +49,8 @@
   }
 }
 
-static std::string spellHeader(const Header &H, HeaderSearch &HS,
-   const FileEntry *Main) {
+std::string spellHeader(const Header &H, HeaderSearch &HS,
+const FileEntry *Main) {
   switch (H.kind()) {
   case Header::Physical: {
 bool IsSystem = false;
Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
===
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
@@ -73,6 +73,8 @@
 std::string fixIncludes(const AnalysisResults &Results, llvm::StringRef Code,
 const format::FormatStyle &IncludeStyle);
 
+std::string spellHeader(const Header &H, HeaderSearch &HS,
+const FileEntry *Main);
 } // namespace include_cleaner
 } // namespace clang
 
Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -665,7 +665,7 @@
 TEST(PreamblePatch, DiagnosticsToPreamble) {
   Config Cfg;
   Cfg.Diagnostics.AllowStalePreamble = true;
-  Cfg.Diagnostics.UnusedIncludes = Config::UnusedIncludesPolicy::Strict;
+  Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict;
   WithContextValue WithCfg(Config::Key, std::move(Cfg));
 
   llvm::StringMap AdditionalFiles;
Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -8,26 +8,56 @@
 
 #include "Annotations.h"
 #include "Config.h"
+#include "Diagnostics.h"
 #include "IncludeCleaner.h"
+#include "ParsedAST.h"
 #include "SourceCode.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "clang-include-cleaner/Analysis.h"
+#include "clang-include-cleaner/Types.h"
 #include "support/Context.h"
+#include "clang/AST/DeclBase.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
 namespace {
 
+using ::testing::AllOf;
 using ::testing::ElementsAre;
 using ::testing::ElementsAreArray;
 using ::testing::IsEmpty;
+using ::testing::Matcher;
 using ::testing::Pointee;
 using ::testing::UnorderedElementsAre;
 
+Matcher withFix(::testing::Matcher FixMatcher) {
+  return Field(&Diag::Fixes, ElementsAre(FixMatcher));
+}
+
+MATCHER_P2(Diag, Range, Message,
+   "Diag at " + llvm::to_string(Range) + " = [" + Message + "]") {
+  return arg.Range == Range && arg.Message == Message;
+}
+
+MATCHER_P3(Fix, Range, Replacement, Message,
+   "Fix " + llvm::to_string(Range) + " => " +
+   ::testing::PrintToString(Replacement) + " = [" + Message + "]") {
+  return arg.Message == Message && arg.Edits.size() == 1 &&
+ arg.Edits[0].range == Range && arg.Edits[0].newText == Replacement;
+}
+
 std::string guard(llvm::StringRef Code) {
   return "#pragma once\n" + Code.str();
 }
@@ -342,7 +372,8 @@
   auto AST = TU.build();
   EXPECT_THAT(computeUnusedIncludes(AST),
   ElementsAre(Pointee(writtenInclusion("";
-  EXPECT_THAT(computeUnusedIncludesExperimental(AST),
+  IncludeCleanerFindings Findings = compu

[PATCH] D143496: [clangd] Add support for missing includes analysis.

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo added a comment.

Thanks for the comments!




Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:712
+  for (auto *Inc : ConvertedIncludes.match(H)) {
+if (Pragmas == nullptr || 
Pragmas->getPublic(Inc->Resolved).empty())
+  Satisfied = true;

kadircet wrote:
> kadircet wrote:
> > you can directly use `!Pragmas->isPrivate(Inc->Resolved)` here, instead of 
> > getpublic
> this check seems to be new. what's the reason for rejecting private 
> providers? I can see that we might want to be conservative by not inserting 
> private providers, but treating symbols as unsatisfied when a private 
> provider is **already** included doesn't feel right. e.g. the code being 
> analyzed might be allowed to depend on this private header, because it's also 
> part of the library, or it's the public header that's exposing this private 
> header. in such a scenario we shouldn't try to insert the public header 
> again. is there a more concrete issue this code is trying to address?
Ok makes sense. No, I guess I was just confused, because I understood that you 
wanted a test that includes "private.h" with a diagnostic generated saying that 
"public.h" should be included instead, so I assumed that was expected 
behaviour. But that's not what you meant, so I misunderstood.



Comment at: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp:449
+include_cleaner::Symbol B{*D};
+syntax::FileRange BRange{SM, B.declaration().getBeginLoc(), 1};
+include_cleaner::Header Header{*SM.getFileManager().getFile("b.h")};

kadircet wrote:
> VitaNuo wrote:
> > kadircet wrote:
> > > this is pointing at the declaration inside `b.h` not to the reference 
> > > inside the main file. are you sure this test passes?
> > Yes, all the tests pass. 
> > `D` is a `Decl` from the main file, otherwise it wouldn't have passed the 
> > safeguard ` if 
> > (!SM.isWrittenInMainFile(SM.getExpansionLoc(D->getLocation( continue;` 
> > above.
> this is passing because `bool BDeclFound;` is uninitialized above, if you set 
> it to `bool BDeclFound = false;` you should see the test fail.
> 
> there's no declaration for `b` inside the main file, it's declared in `b.h` 
> and **referenced** inside the main file. you still need to search for the 
> decl (without the constraint of being written in main file), use it to build 
> an include_cleaner::Symbol, and use a `clangd::Annotation` range for the 
> range of the reference.
> 
> it might be easer to write this as:
> ```
> const NamedDecl* B = nullptr;
> for (...) {
>  ...
>  B = D;
> }
> ASSERT_TRUE(B);
> // build expected diagnostic info based on B and check that it's equal to 
> what we've produced
> ```
Didn't know there was a difference between uninitialized and `false`..

Thanks for the idea with `ASSERT_TRUE(Decl)`.  Please check out the new version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143496

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

[PATCH] D143984: [DebugMetadata] Simplify handling subprogram's retainedNodes field. NFCI (1/7)

[Juan Manuel Martinez Caamaño via Phabricator via cfe-commits]

jmmartinez added a comment.

Sorry for the delay, I was out-of-office. Patches 1 to 4 look good to me. If 
nobody raises concerns this week I'll mark them as accepted.

I'm currently reviewing patch 5.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143984

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

[PATCH] D145486: [clang] Fix single-element array initialization in constexpr

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 503020.
Fznamznon added a comment.

Rebase and update test comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145486

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constexpr-single-element-array.cpp


Index: clang/test/SemaCXX/constexpr-single-element-array.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constexpr-single-element-array.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+
+// This test makes sure that a single element array doesn't produce
+// spurious errors during constexpr evaluation.
+
+// expected-no-diagnostics
+struct Sub { int x; };
+
+struct S {
+  constexpr S() { Arr[0] = Sub{}; }
+  Sub Arr[1];
+};
+
+constexpr bool test() {
+  S s;
+  return true;
+}
+
+static_assert(test());
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -10917,7 +10917,7 @@
 for (unsigned I = OldElts; I < N; ++I)
   Value->getArrayInitializedElt(I) = Filler;
 
-  if (HasTrivialConstructor && N == FinalSize) {
+  if (HasTrivialConstructor && N == FinalSize && FinalSize != 1) {
 // If we have a trivial constructor, only evaluate it once and copy
 // the result into all the array elements.
 APValue &FirstResult = Value->getArrayInitializedElt(0);


Index: clang/test/SemaCXX/constexpr-single-element-array.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constexpr-single-element-array.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+
+// This test makes sure that a single element array doesn't produce
+// spurious errors during constexpr evaluation.
+
+// expected-no-diagnostics
+struct Sub { int x; };
+
+struct S {
+  constexpr S() { Arr[0] = Sub{}; }
+  Sub Arr[1];
+};
+
+constexpr bool test() {
+  S s;
+  return true;
+}
+
+static_assert(test());
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -10917,7 +10917,7 @@
 for (unsigned I = OldElts; I < N; ++I)
   Value->getArrayInitializedElt(I) = Filler;
 
-  if (HasTrivialConstructor && N == FinalSize) {
+  if (HasTrivialConstructor && N == FinalSize && FinalSize != 1) {
 // If we have a trivial constructor, only evaluate it once and copy
 // the result into all the array elements.
 APValue &FirstResult = Value->getArrayInitializedElt(0);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130303: Handle template parameter-dependent bit field widths in libclang

[Igor Kushnir via Phabricator via cfe-commits]

vedgy added inline comments.



Comment at: clang/include/clang-c/Index.h:3552
+ * If the cursor does not reference a bit field declaration or if the bit
+ * field's width does not depend on template parameters, 0 is returned.
+ */

vedgy wrote:
> collinbaker wrote:
> > vedgy wrote:
> > > I just thought how the new API could be used in KDevelop. Currently when 
> > > `clang_getFieldDeclBitWidth()` is positive, e.g. 2, KDevelop shows  ` : 
> > > 2` after the data member name in a tooltip. Ideally a 
> > > template-param-dependent expression (actual code) would be displayed 
> > > after the colon. If that's difficult to implement, `: [tparam-dependent]` 
> > > or `: ?` could be displayed instead. But it would be more convenient and 
> > > efficient to get this information by a single call to 
> > > `clang_getFieldDeclBitWidth()` instead of calling 
> > > `clang_isFieldDeclBitWidthDependent()` each time 
> > > `clang_getFieldDeclBitWidth()` returns `-1`. So how about returning `-2` 
> > > or `0` from `clang_getFieldDeclBitWidth()` instead of adding this new API?
> > I understand the motivation but I don't think requiring an extra call is 
> > asking too much of libclang clients, and it's one extra call that doesn't 
> > do much work and will be called rarely so I don't see efficiency concerns. 
> > Without strong reasons otherwise I think it's better to be explicit here.
> KDevelop calls `clang_getFieldDeclBitWidth()` for each encountered class 
> member declaration. `clang_isFieldDeclBitWidthDependent()` would have to be 
> called each time `clang_getFieldDeclBitWidth()` returns `-1`, which would be 
> most of the time, because few class members are bit-fields. The work this new 
> function does is the same as that of `clang_getFieldDeclBitWidth()`  
> (repeated).
> 
> If the concern about returning `-2` from `clang_getFieldDeclBitWidth()` is 
> cryptic return codes, an `enum` with named constants can be introduced.
> 
> If the concern is breaking backward compatibility for users that relied on 
> the returned value being positive or `-1`, then a replacement for 
> `clang_getFieldDeclBitWidth()` with the most convenient API should be 
> introduced and `clang_getFieldDeclBitWidth()` itself - deprecated.
> 
> KDevelop simply stores the value returned by `clang_getFieldDeclBitWidth()` 
> in an `int16_t m_bitWidth` data member and uses it later. So if `-2` is 
> returned, the only place in code to adjust would be the use of this data 
> member. With the current `clang_isFieldDeclBitWidthDependent()` 
> implementation, this function would have to be called, `-2` explicitly stored 
> in `m_bitWidth` and the use of `m_bitWidth` would have to be adjusted just 
> the same.
> 
> Have you considered potential usage of the added API in your project? Which 
> alternative would be more convenient to use?
One more API alternative is to replace `clang_isFieldDeclBitWidthDependent()` 
with `clang_isBitFieldDecl()`. The usage would be more straightforward and 
efficient: first call `clang_isBitFieldDecl()`; if it returns true (should be 
rare enough), call `clang_getFieldDeclBitWidth()`; if that returns `-1`, then 
the bit-field width must be unknown (dependent on template parameters). Such 
usage would still be less convenient and efficient compared to 
`clang_getFieldDeclBitWidth()` returning `-2` though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130303

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

[PATCH] D127812: [AArch64] FMV support and necessary target features dependencies.

[Pavel Iliin via Phabricator via cfe-commits]

ilinpv added inline comments.



Comment at: compiler-rt/lib/builtins/cpu_model.c:1338
+  hwcap = getauxval(AT_HWCAP);
+  hwcap2 = getauxval(AT_HWCAP2);
+#endif // defined(__FreeBSD__)

nikic wrote:
> This breaks the build with glibc 2.17.
Thanks for https://reviews.llvm.org/D145494 fix. AT_HWCAP2 was not defined.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127812

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

[PATCH] D143496: [clangd] Add support for missing includes analysis.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:456
+  auto IncludeStyle =
+  clang::format::getLLVMStyle().IncludeStyle;
+  auto DefaultStyle = clang::format::getStyle(

creating a copy of LLVM style unnecessarily all the time is not really great, 
can you move this into the failure case instead?

also you can drop the `clang::` here and elsewhere, as this code is already 
part of `clang::` namespace.



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:457
+  clang::format::getLLVMStyle().IncludeStyle;
+  auto DefaultStyle = clang::format::getStyle(
+  clang::format::DefaultFormatStyle, MainFile->getName(),

as mentioned above we also need to make sure we're passing the relevant VFS 
instance inside the source manager, rather than using the real file system (as 
some clients rely on the VFS).



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:458
+  auto DefaultStyle = clang::format::getStyle(
+  clang::format::DefaultFormatStyle, MainFile->getName(),
+  clang::format::DefaultFallbackStyle);

s/MainFile->getName()/AST.tuPath()/

to be consistent with other places.



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:460
+  clang::format::DefaultFallbackStyle);
+  if (!DefaultStyle.takeError()) {
+IncludeStyle = DefaultStyle->IncludeStyle;

can you also `elog` this error? as it should be rare and when this goes wrong, 
having this mentioned in the logs are really useful for debugging (since the 
failure is actually outside of clangd, it usually means a malformed config file 
somewhere)



Comment at: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp:426
+void foo() {
+  ^b();
+})cpp");

nit: instead of using a point, can you use a range here instead (i.e. `[[b]]`)? 
afterwards you can have a `FileRange` pointing at both offsets, rather than 
relying on the length of the identifier.



Comment at: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp:446
+BDecl = CandidateDecl;
+include_cleaner::Symbol B{*D};
+auto Offset = positionToOffset(MainFile.code(), MainFile.point());

rest of the code here doesn't really belong to the for loop, can you take them 
out?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143496

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

[PATCH] D145491: [clang] Replace Member Expressions During Instantiation If Necessary

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Other than the two auto's missing a *, I think this is alright.  I want to 
think about it/give others a chance to review, so please ping this in a few 
days if it hasn't been accepted by then.




Comment at: clang/lib/Sema/TreeTransform.h:2808
+isa(Member)) {
+  if (auto ThisClass = cast(Base)
+   ->getType()





Comment at: clang/lib/Sema/TreeTransform.h:2812
+   ->getAsCXXRecordDecl()) {
+auto Class = cast(Member->getDeclContext());
+// In unevaluated contexts, an expression supposed to be a member 
access




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145491

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

[PATCH] D143418: [libclang] Add API to override preamble storage path

[Igor Kushnir via Phabricator via cfe-commits]

vedgy added a comment.

A clang-ppc64le-rhel build 
 failed:

  54.897 [28/169/148] Building CXX object 
tools/clang/unittests/libclang/CMakeFiles/libclangTests.dir/LibclangTest.cpp.o
  FAILED: 
tools/clang/unittests/libclang/CMakeFiles/libclangTests.dir/LibclangTest.cpp.o 
  /home/buildbots/clang.15.0.4/bin/clang++ --gcc-toolchain=/usr 
-DGTEST_HAS_RTTI=0 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE 
-D_LIBCPP_ENABLE_ASSERTIONS -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS 
-D__STDC_LIMIT_MACROS 
-I/home/buildbots/docker-RHEL84-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/build/tools/clang/unittests/libclang
 
-I/home/buildbots/docker-RHEL84-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang/unittests/libclang
 
-I/home/buildbots/docker-RHEL84-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang/include
 
-I/home/buildbots/docker-RHEL84-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/build/tools/clang/include
 
-I/home/buildbots/docker-RHEL84-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/build/include
 
-I/home/buildbots/docker-RHEL84-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/include
 
-I/home/buildbots/docker-RHEL84-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/third-party/unittest/googletest/include
 
-I/home/buildbots/docker-RHEL84-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/third-party/unittest/googlemock/include
 -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror 
-Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra 
-Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers 
-pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough 
-Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
-Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion 
-Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color 
-ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual 
-Wno-nested-anon-types -O3 -DNDEBUG  -Wno-variadic-macros 
-Wno-gnu-zero-variadic-macro-arguments -fno-exceptions -funwind-tables 
-fno-rtti -UNDEBUG -Wno-suggest-override -std=c++17 -MD -MT 
tools/clang/unittests/libclang/CMakeFiles/libclangTests.dir/LibclangTest.cpp.o 
-MF 
tools/clang/unittests/libclang/CMakeFiles/libclangTests.dir/LibclangTest.cpp.o.d
 -o 
tools/clang/unittests/libclang/CMakeFiles/libclangTests.dir/LibclangTest.cpp.o 
-c 
/home/buildbots/docker-RHEL84-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang/unittests/libclang/LibclangTest.cpp
  
/home/buildbots/docker-RHEL84-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang/unittests/libclang/LibclangTest.cpp:488:50:
 error: missing field 'ThreadBackgroundPriorityForIndexing' initializer 
[-Werror,-Wmissing-field-initializers]
  CXIndexOptions Opts = {sizeof(CXIndexOptions)};
   ^
  1 error generated.

Same on ppc64le-lld-multistage-test 
.

Do you know why partial struct initialization is unsupported on this platform?
A simple workaround is to use `memset` in this single place. I am preparing a 
patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143418

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

[clang] df8f8f7 - Fix build failures with libclang unittest; NFC

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2023-03-07T09:32:51-05:00
New Revision: df8f8f76207df40dca11c9c0c2328d6b3dfba9ca

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

LOG: Fix build failures with libclang unittest; NFC

This addresses the issue found by:
https://lab.llvm.org/buildbot/#/builders/57/builds/25217
https://lab.llvm.org/buildbot/#/builders/36/builds/31018

Added: 


Modified: 
clang/unittests/libclang/LibclangTest.cpp

Removed: 




diff  --git a/clang/unittests/libclang/LibclangTest.cpp 
b/clang/unittests/libclang/LibclangTest.cpp
index 18d0fc12a51cf..662377cd0d581 100644
--- a/clang/unittests/libclang/LibclangTest.cpp
+++ b/clang/unittests/libclang/LibclangTest.cpp
@@ -485,7 +485,8 @@ class LibclangSetPreambleStoragePathTest : public 
LibclangPreambleStorageTest {
   void CreateIndex() override {
 InitializePreambleDir();
 
-CXIndexOptions Opts = {sizeof(CXIndexOptions)};
+CXIndexOptions Opts{};
+Opts.Size = sizeof(CXIndexOptions);
 Opts.PreambleStoragePath = PreambleStoragePath();
 Index = clang_createIndexWithOptions(&Opts);
 ASSERT_TRUE(Index);



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

[PATCH] D143418: [libclang] Add API to override preamble storage path

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D143418#4175128 , @vedgy wrote:

> A clang-ppc64le-rhel build 
>  failed:
>
>   54.897 [28/169/148] Building CXX object 
> tools/clang/unittests/libclang/CMakeFiles/libclangTests.dir/LibclangTest.cpp.o
>   FAILED: 
> tools/clang/unittests/libclang/CMakeFiles/libclangTests.dir/LibclangTest.cpp.o
>  
>   /home/buildbots/clang.15.0.4/bin/clang++ --gcc-toolchain=/usr 
> -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE 
> -D_LIBCPP_ENABLE_ASSERTIONS -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS 
> -D__STDC_LIMIT_MACROS 
> -I/home/buildbots/docker-RHEL84-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/build/tools/clang/unittests/libclang
>  
> -I/home/buildbots/docker-RHEL84-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang/unittests/libclang
>  
> -I/home/buildbots/docker-RHEL84-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang/include
>  
> -I/home/buildbots/docker-RHEL84-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/build/tools/clang/include
>  
> -I/home/buildbots/docker-RHEL84-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/build/include
>  
> -I/home/buildbots/docker-RHEL84-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/include
>  
> -I/home/buildbots/docker-RHEL84-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/third-party/unittest/googletest/include
>  
> -I/home/buildbots/docker-RHEL84-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/third-party/unittest/googlemock/include
>  -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror 
> -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra 
> -Wno-unused-parameter -Wwrite-strings -Wcast-qual 
> -Wmissing-field-initializers -pedantic -Wno-long-long 
> -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default 
> -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor 
> -Wsuggest-override -Wstring-conversion -Wmisleading-indentation 
> -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections 
> -fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 
> -DNDEBUG  -Wno-variadic-macros -Wno-gnu-zero-variadic-macro-arguments 
> -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -Wno-suggest-override 
> -std=c++17 -MD -MT 
> tools/clang/unittests/libclang/CMakeFiles/libclangTests.dir/LibclangTest.cpp.o
>  -MF 
> tools/clang/unittests/libclang/CMakeFiles/libclangTests.dir/LibclangTest.cpp.o.d
>  -o 
> tools/clang/unittests/libclang/CMakeFiles/libclangTests.dir/LibclangTest.cpp.o
>  -c 
> /home/buildbots/docker-RHEL84-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang/unittests/libclang/LibclangTest.cpp
>   
> /home/buildbots/docker-RHEL84-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang/unittests/libclang/LibclangTest.cpp:488:50:
>  error: missing field 'ThreadBackgroundPriorityForIndexing' initializer 
> [-Werror,-Wmissing-field-initializers]
>   CXIndexOptions Opts = {sizeof(CXIndexOptions)};
>^
>   1 error generated.
>
> Same on ppc64le-lld-multistage-test 
> .
>
> Do you know why partial struct initialization is unsupported on this platform?
> A simple workaround is to use `memset` in this single place. I am preparing a 
> patch.

It's a two-stage build. The first stage passed, but the second stage is built 
with `-Werror`, which is why it fails. I addressed the issue in 
df8f8f76207df40dca11c9c0c2328d6b3dfba9ca 
, so no 
need for you to worry about making a patch. I went with `{}` to perform the 
zero initialization instead of `memset`, but either should work fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143418

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

[PATCH] D145057: [clang][ASTImport] Add support for import of empty records

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

balazske added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:3897
+  if (Err)
+return std::move(Err);
   ToField->setAccess(D->getAccess());

kpdev42 wrote:
> balazske wrote:
> > I am not familiar with this use case, is there a path where the attributes 
> > are read from a `FieldDecl` before return from `VisitFieldDecl`? Probably 
> > `ImportImpl` is overridden? Importing the attributes here should work but 
> > not totally sure if it does not cause problems. Problematic case is if the 
> > attribute has pointer to a `Decl` or `Type` that is imported here in a 
> > state when the field is already created but not initialized. Another 
> > problem is that attributes are added a second time in `Import(Decl *)`. Can 
> > it work if the `ImportAttrs` is made a protected function and called from 
> > (overridden) `ImportImpl` (still there can be a second import in 
> > `Import(Decl *)`?
> The problem is that field attributes are required when we are adding a copy 
> of a field to a structure in `VisitFieldDecl`. Attributes themselves are read 
> in `CXXRecordDecl::addedMember` (see the call to `isZeroSize`). Adding them 
> after `ImportImpl` is called is too late: record is already marked as 
> non-empty. 
I understand now. But a comment could be added here to explain why the 
attribute import is needed here.



Comment at: clang/lib/AST/ASTImporter.cpp:8985
+Error ASTImporter::ImportAttrs(Decl *ToD, Decl *FromD) {
+  if (FromD->hasAttrs())
+for (const Attr *FromAttr : FromD->getAttrs()) {

Should use braces in this case 
(https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145057

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

[PATCH] D145057: [clang][ASTImport] Add support for import of empty records

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

balazske added inline comments.



Comment at: clang/include/clang/AST/DeclCXX.h:1171
+  /// attribute
+  void markEmpty() { data().Empty = true; }
 

This change looks not related.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145057

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

[clang] 07158c5 - [Clang] Create opaque type for AArch64 SVE2p1/SME2 svcount_t.

[Sander de Smalen via cfe-commits]

Author: Sander de Smalen
Date: 2023-03-07T14:43:50Z
New Revision: 07158c54add927057690aa8c073d35d42eac7006

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

LOG: [Clang] Create opaque type for AArch64 SVE2p1/SME2 svcount_t.

This patch adds the builtin type __SVCount_t to Clang, which is an opaque
scalable type defined in the SME2 C and C++ Language Extensions.

The type maps to the `target("aarch64.svcount")` LLVM IR type.

Reviewed By: paulwalker-arm

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

Added: 


Modified: 
clang/include/clang/AST/Type.h
clang/include/clang/Basic/AArch64SVEACLETypes.def
clang/lib/AST/ASTContext.cpp
clang/lib/AST/ItaniumMangle.cpp
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CodeGenTypes.cpp
clang/test/AST/ast-dump-aarch64-sve-types.c
clang/test/CodeGen/aarch64-debug-sve-vector-types.c
clang/test/CodeGen/aarch64-sve-inline-asm-crash.c
clang/test/CodeGenCXX/aarch64-mangle-sve-vectors.cpp
clang/test/CodeGenCXX/aarch64-sve-typeinfo.cpp
clang/unittests/AST/SizelessTypesTest.cpp

Removed: 




diff  --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 10cb4c819d7ce..95a5df8699afb 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -2700,6 +2700,8 @@ class BuiltinType : public Type {
 
   bool isSVEBool() const { return getKind() == Kind::SveBool; }
 
+  bool isSVECount() const { return getKind() == Kind::SveCount; }
+
   /// Determines whether the given kind corresponds to a placeholder type.
   static bool isPlaceholderTypeKind(Kind K) {
 return K >= Overload;

diff  --git a/clang/include/clang/Basic/AArch64SVEACLETypes.def 
b/clang/include/clang/Basic/AArch64SVEACLETypes.def
index b98a07436e941..cb2f673af06d5 100644
--- a/clang/include/clang/Basic/AArch64SVEACLETypes.def
+++ b/clang/include/clang/Basic/AArch64SVEACLETypes.def
@@ -49,6 +49,11 @@
   SVE_TYPE(Name, Id, SingletonId)
 #endif
 
+#ifndef SVE_OPAQUE_TYPE
+#define SVE_OPAQUE_TYPE(Name, MangledName, Id, SingletonId)
\
+  SVE_TYPE(Name, Id, SingletonId)
+#endif
+
 //===- Vector point types ---===//
 
 
@@ -125,6 +130,9 @@ SVE_VECTOR_TYPE("__clang_svbfloat16x4_t", "svbfloat16x4_t", 
SveBFloat16x4, SveBF
 
 SVE_PREDICATE_TYPE("__SVBool_t", "__SVBool_t", SveBool, SveBoolTy, 16)
 
+SVE_OPAQUE_TYPE("__SVCount_t", "__SVCount_t", SveCount, SveCountTy)
+
 #undef SVE_VECTOR_TYPE
 #undef SVE_PREDICATE_TYPE
+#undef SVE_OPAQUE_TYPE
 #undef SVE_TYPE

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 499009b54a04e..1995030eec25f 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -2322,6 +2322,11 @@ TypeInfo ASTContext::getTypeInfoImpl(const Type *T) 
const {
 Width = 0; 
\
 Align = 16;
\
 break;
+#define SVE_OPAQUE_TYPE(Name, MangledName, Id, SingletonId)
\
+  case BuiltinType::Id:
\
+Width = 0; 
\
+Align = 16;
\
+break;
 #include "clang/Basic/AArch64SVEACLETypes.def"
 #define PPC_VECTOR_TYPE(Name, Id, Size)
\
   case BuiltinType::Id:
\
@@ -4119,6 +4124,7 @@ QualType ASTContext::getScalableVectorType(QualType EltTy,
 #define SVE_PREDICATE_TYPE(Name, MangledName, Id, SingletonId, NumEls) 
\
   if (EltTy->isBooleanType() && NumElts == NumEls) 
\
 return SingletonId;
+#define SVE_OPAQUE_TYPE(Name, MangledName, Id, SingleTonId)
 #include "clang/Basic/AArch64SVEACLETypes.def"
   } else if (Target->hasRISCVVTypes()) {
 uint64_t EltTySize = getTypeSize(EltTy);
@@ -9549,9 +9555,10 @@ bool ASTContext::areCompatibleVectorTypes(QualType 
FirstVec,
 /// getSVETypeSize - Return SVE vector or predicate register size.
 static uint64_t getSVETypeSize(ASTContext &Context, const BuiltinType *Ty) {
   assert(Ty->isVLSTBuiltinType() && "Invalid SVE Type");
-  return Ty->getKind() == BuiltinType::SveBool
- ? (Context.getLangOpts().VScaleMin * 128) / Context.getCharWidth()
- : Context.getLangOpts().VScaleMin * 128;
+  if (Ty->getKind() == BuiltinType::SveBool ||
+  Ty->getKind() == BuiltinType::SveCount)
+return (Context.getLangOpts().VScaleMin * 128) / Context.getCharWidth();
+  return Context.getLangOpts().VScaleMin * 128;
 }
 
 bool ASTContext::areCompatibleSveTypes(Qual

[PATCH] D136864: [Clang] Create opaque type for AArch64 SVE2p1/SME2 svcount_t.

[Sander de Smalen 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 rG07158c54add9: [Clang] Create opaque type for AArch64 
SVE2p1/SME2 svcount_t. (authored by sdesmalen).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136864

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/AArch64SVEACLETypes.def
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/test/AST/ast-dump-aarch64-sve-types.c
  clang/test/CodeGen/aarch64-debug-sve-vector-types.c
  clang/test/CodeGen/aarch64-sve-inline-asm-crash.c
  clang/test/CodeGenCXX/aarch64-mangle-sve-vectors.cpp
  clang/test/CodeGenCXX/aarch64-sve-typeinfo.cpp
  clang/unittests/AST/SizelessTypesTest.cpp

Index: clang/unittests/AST/SizelessTypesTest.cpp
===
--- clang/unittests/AST/SizelessTypesTest.cpp
+++ clang/unittests/AST/SizelessTypesTest.cpp
@@ -45,6 +45,7 @@
   ASSERT_TRUE(Ctx.SveBFloat16Ty->isSizelessBuiltinType());
 
   ASSERT_TRUE(Ctx.SveBoolTy->isSizelessBuiltinType());
+  ASSERT_TRUE(Ctx.SveCountTy->isSizelessBuiltinType());
 
   ASSERT_FALSE(Ctx.VoidTy->isSizelessBuiltinType());
   ASSERT_FALSE(Ctx.PseudoObjectTy->isSizelessBuiltinType());
@@ -55,6 +56,12 @@
   Ctx.getLValueReferenceType(Ctx.SveBoolTy)->isSizelessBuiltinType());
   ASSERT_FALSE(
   Ctx.getRValueReferenceType(Ctx.SveBoolTy)->isSizelessBuiltinType());
+
+  ASSERT_FALSE(Ctx.getPointerType(Ctx.SveCountTy)->isSizelessBuiltinType());
+  ASSERT_FALSE(
+  Ctx.getLValueReferenceType(Ctx.SveCountTy)->isSizelessBuiltinType());
+  ASSERT_FALSE(
+  Ctx.getRValueReferenceType(Ctx.SveCountTy)->isSizelessBuiltinType());
 }
 
 TEST_F(SizelessTypeTester, TestSizeless) {
@@ -75,6 +82,7 @@
   ASSERT_TRUE(Ctx.SveBFloat16Ty->isSizelessType());
 
   ASSERT_TRUE(Ctx.SveBoolTy->isSizelessType());
+  ASSERT_TRUE(Ctx.SveCountTy->isSizelessType());
 
   ASSERT_FALSE(Ctx.VoidTy->isSizelessType());
   ASSERT_FALSE(Ctx.PseudoObjectTy->isSizelessType());
@@ -83,4 +91,8 @@
   ASSERT_FALSE(Ctx.getPointerType(Ctx.SveBoolTy)->isSizelessType());
   ASSERT_FALSE(Ctx.getLValueReferenceType(Ctx.SveBoolTy)->isSizelessType());
   ASSERT_FALSE(Ctx.getRValueReferenceType(Ctx.SveBoolTy)->isSizelessType());
+
+  ASSERT_FALSE(Ctx.getPointerType(Ctx.SveCountTy)->isSizelessType());
+  ASSERT_FALSE(Ctx.getLValueReferenceType(Ctx.SveCountTy)->isSizelessType());
+  ASSERT_FALSE(Ctx.getRValueReferenceType(Ctx.SveCountTy)->isSizelessType());
 }
Index: clang/test/CodeGenCXX/aarch64-sve-typeinfo.cpp
===
--- clang/test/CodeGenCXX/aarch64-sve-typeinfo.cpp
+++ clang/test/CodeGenCXX/aarch64-sve-typeinfo.cpp
@@ -22,6 +22,7 @@
 auto &bf16 = typeid(__SVBFloat16_t);
 
 auto &b8 = typeid(__SVBool_t);
+auto &c8 = typeid(__SVCount_t);
 
 // CHECK-DAG: @_ZTSu10__SVInt8_t = {{.*}} c"u10__SVInt8_t\00"
 // CHECK-DAG: @_ZTIu10__SVInt8_t = {{.*}} @_ZTVN10__cxxabiv123__fundamental_type_infoE, {{.*}} @_ZTSu10__SVInt8_t
@@ -61,3 +62,6 @@
 
 // CHECK-DAG: @_ZTSu10__SVBool_t = {{.*}} c"u10__SVBool_t\00"
 // CHECK-DAG: @_ZTIu10__SVBool_t = {{.*}} @_ZTVN10__cxxabiv123__fundamental_type_infoE, {{.*}} @_ZTSu10__SVBool_t
+
+// CHECK-DAG: @_ZTSu11__SVCount_t = {{.*}} c"u11__SVCount_t\00"
+// CHECK-DAG: @_ZTIu11__SVCount_t = {{.*}} @_ZTVN10__cxxabiv123__fundamental_type_infoE, {{.*}} @_ZTSu11__SVCount_t
Index: clang/test/CodeGenCXX/aarch64-mangle-sve-vectors.cpp
===
--- clang/test/CodeGenCXX/aarch64-mangle-sve-vectors.cpp
+++ clang/test/CodeGenCXX/aarch64-mangle-sve-vectors.cpp
@@ -31,6 +31,8 @@
 void f12(S<__SVBFloat16_t>) {}
 // CHECK: _Z3f131SIu10__SVBool_tE
 void f13(S<__SVBool_t>) {}
+// CHECK: _Z3f141SIu11__SVCount_tE
+void f14(S<__SVCount_t>) {}
 
 // The tuple types don't use the internal name for mangling.
 
Index: clang/test/CodeGen/aarch64-sve-inline-asm-crash.c
===
--- clang/test/CodeGen/aarch64-sve-inline-asm-crash.c
+++ clang/test/CodeGen/aarch64-sve-inline-asm-crash.c
@@ -20,5 +20,17 @@
   return ret ;
 }
 
+__SVCount_t funcB1(__SVCount_t in)
+{
+  __SVCount_t ret ;
+  asm volatile (
+"mov %[ret].b, %[in].b \n"
+: [ret] "=w" (ret)
+: [in] "w" (in)
+:);
+
+  return ret ;
+}
+
 // CHECK: funcB1
 // CHECK-ERROR: fatal error: error in backend: Cannot select
Index: clang/test/CodeGen/aarch64-debug-sve-vector-types.c
===
--- clang/test/CodeGen/aarch64-debug-sve-vector-types.c
+++ clang/test/CodeGen/aarch64-debug-sve-vector-types.c
@@ -9,6 +9,12 @@
   // CHECK-DAG: ![[REALELTS1_64]] = !DISubrange(lowerBound: 0, upperBound: !DIExpression(DW_OP_constu, 1,

[PATCH] D129951: adds `__disable_adl` attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: erichkeane, clang-language-wg.
aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:4132
+def DisableADL : InheritableAttr {
+  let Spellings = [Keyword<"__disable_adl">];
+  let Subjects = SubjectList<[Function]>;

cjdb wrote:
> rsmith wrote:
> > Has this syntax been discussed already? If not, why did you choose keyword 
> > syntax instead of a normal attribute?
> Aaron and I discussed this offline. I'd originally intended to do 
> `[[clang::disable_adl]]`, but Aaron pointed out that this would be 
> problematic for all involved if someone uses a library in GCC, since the 
> feature is implicitly turned off. If we use this syntax, then it forces 
> libraries to acknowledge (and possibly document) that ADL is restricted on 
> Clang, but potentially not GCC.
> 
> I will add a note to the documentation that I'm promising.
Yup! The basic thinking I had is: if this attribute is ignored by an 
implementation, it's very possible that the user's code may change behavior -- 
if they're unlucky, it's a silent behavior change. However, if it's a keyword, 
then there's a syntax error and no chance for a silent behavior change.

If you think we need a more broad discussion of the syntax, that'd be fine by 
me -- it's possible there's a policy question hiding in here about when we want 
keywords and when we are fine with regular attribute syntax (type attributes 
are another source of these kinds of questions).



Comment at: clang/include/clang/Basic/AttrDocs.td:6851
+def DisableADLDocs : Documentation {
+  let Content = [{Please don't LGTM without this being fully documented.}];
+}

rsmith wrote:
> OK!
Btw, as a reviewer: thank you for this. I appreciate clear markings of "we're 
not done yet!" in the source.



Comment at: clang/include/clang/Basic/AttrDocs.td:542
 If a statement is marked ``nomerge`` and contains call expressions, those call
-expressions inside the statement will not be merged during optimization. This 
+expressions inside the statement will not be merged during optimization. This
 attribute can be used to prevent the optimizer from obscuring the source

cjdb wrote:
> shafik wrote:
> > A lot of these look like unrelated changes.
> I tend to not argue with autoformatters.
We still ask that you don't commit unrelated changes (this was stripping 
trailing whitespace, which is often an editor setting) -- it makes git blame 
harder than it needs to be. Feel free to commit the whitespace fixes 
before/after the patch if you'd like, though!



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5756
+  if (FunctionDecl *F = D->getAsFunction();
+  F->isOverloadedOperator() || F->isCXXClassMember()) {
+S.Diag(AL.getLoc(), diag::err_disable_adl_no_operators)

cjdb wrote:
> rsmith wrote:
> > Maybe also global `operator new` / `operator delete`?
> How does ADL even work in with the global namespace? Should it be invalid to 
> apply here?
Should this be `isCXXInstanceMember()` instead? Or do we intend to disallow 
this on static member functions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129951

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

[PATCH] D145173: Make section attribute and -ffunction-sections play nicely

[Paul Robinson via Phabricator via cfe-commits]

probinson abandoned this revision.
probinson added a comment.

I think the GC behavior with explicit section names is currently a little 
peculiar. For functions without a section name, -ffunction-sections allows GC 
to happen at the individual function level. With a section name, GC would 
happen at the level of all-or-nothing per input file, regardless of 
-ffunction-sections. That just seems unexpected and inconsistent to me.
But it is the current behavior, and given the objections it's not worth 
pursuing this.


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

https://reviews.llvm.org/D145173

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

[PATCH] D143418: [libclang] Add API to override preamble storage path

[Igor Kushnir via Phabricator via cfe-commits]

vedgy added a comment.

Thank you for the quick build fix. KDevelop's CMakeLists.txt disables this 
warning by adding the `-Wno-missing-field-initializers` flag. That's why I 
haven't noticed the warning while building KDevelop. Either I missed the 
warning while building LLVM or it is also disabled in a default GNU/Linux 
x86_64 build.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143418

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

[PATCH] D129951: adds `__disable_adl` attribute

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5756
+  if (FunctionDecl *F = D->getAsFunction();
+  F->isOverloadedOperator() || F->isCXXClassMember()) {
+S.Diag(AL.getLoc(), diag::err_disable_adl_no_operators)

aaron.ballman wrote:
> cjdb wrote:
> > rsmith wrote:
> > > Maybe also global `operator new` / `operator delete`?
> > How does ADL even work in with the global namespace? Should it be invalid 
> > to apply here?
> Should this be `isCXXInstanceMember()` instead? Or do we intend to disallow 
> this on static member functions?
Static member functions would be qualified, and not involve ADL. This seems 
correct.



Comment at: clang/lib/Sema/SemaOverload.cpp:13972-13973
   // Mark member== const or provide matching != to disallow 
reversed
-  // args. Eg. 
-  // struct S { bool operator==(const S&); }; 
+  // args. Eg.
+  // struct S { bool operator==(const S&); };
   // S()==S();

White space only changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129951

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

[PATCH] D129951: adds `__disable_adl` attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5756
+  if (FunctionDecl *F = D->getAsFunction();
+  F->isOverloadedOperator() || F->isCXXClassMember()) {
+S.Diag(AL.getLoc(), diag::err_disable_adl_no_operators)

cor3ntin wrote:
> aaron.ballman wrote:
> > cjdb wrote:
> > > rsmith wrote:
> > > > Maybe also global `operator new` / `operator delete`?
> > > How does ADL even work in with the global namespace? Should it be invalid 
> > > to apply here?
> > Should this be `isCXXInstanceMember()` instead? Or do we intend to disallow 
> > this on static member functions?
> Static member functions would be qualified, and not involve ADL. This seems 
> correct.
Hmmm, I think they're *normally* qualified, but if you call the static function 
from within a member function, it doesn't have to be qualified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129951

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

[PATCH] D129951: adds `__disable_adl` attribute

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5756
+  if (FunctionDecl *F = D->getAsFunction();
+  F->isOverloadedOperator() || F->isCXXClassMember()) {
+S.Diag(AL.getLoc(), diag::err_disable_adl_no_operators)

aaron.ballman wrote:
> cor3ntin wrote:
> > aaron.ballman wrote:
> > > cjdb wrote:
> > > > rsmith wrote:
> > > > > Maybe also global `operator new` / `operator delete`?
> > > > How does ADL even work in with the global namespace? Should it be 
> > > > invalid to apply here?
> > > Should this be `isCXXInstanceMember()` instead? Or do we intend to 
> > > disallow this on static member functions?
> > Static member functions would be qualified, and not involve ADL. This seems 
> > correct.
> Hmmm, I think they're *normally* qualified, but if you call the static 
> function from within a member function, it doesn't have to be qualified.
nvm, they can be found by adl from within member functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129951

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

[PATCH] D143418: [libclang] Add API to override preamble storage path

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D143418#4175188 , @vedgy wrote:

> Thank you for the quick build fix.

You're welcome!

> KDevelop's CMakeLists.txt disables this warning by adding the 
> `-Wno-missing-field-initializers` flag. That's why I haven't noticed the 
> warning while building KDevelop. Either I missed the warning while building 
> LLVM or it is also disabled in a default GNU/Linux x86_64 build.

https://github.com/llvm/llvm-project/blob/main/llvm/cmake/modules/HandleLLVMOptions.cmake#L730


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143418

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

[PATCH] D143496: [clangd] Add support for missing includes analysis.

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo updated this revision to Diff 503033.
VitaNuo marked 6 inline comments as done.
VitaNuo added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143496

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
  clang-tools-extra/clangd/unittests/PreambleTests.cpp
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
  clang-tools-extra/include-cleaner/lib/Analysis.cpp

Index: clang-tools-extra/include-cleaner/lib/Analysis.cpp
===
--- clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -49,8 +49,8 @@
   }
 }
 
-static std::string spellHeader(const Header &H, HeaderSearch &HS,
-   const FileEntry *Main) {
+std::string spellHeader(const Header &H, HeaderSearch &HS,
+const FileEntry *Main) {
   switch (H.kind()) {
   case Header::Physical: {
 bool IsSystem = false;
Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
===
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
@@ -73,6 +73,8 @@
 std::string fixIncludes(const AnalysisResults &Results, llvm::StringRef Code,
 const format::FormatStyle &IncludeStyle);
 
+std::string spellHeader(const Header &H, HeaderSearch &HS,
+const FileEntry *Main);
 } // namespace include_cleaner
 } // namespace clang
 
Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -665,7 +665,7 @@
 TEST(PreamblePatch, DiagnosticsToPreamble) {
   Config Cfg;
   Cfg.Diagnostics.AllowStalePreamble = true;
-  Cfg.Diagnostics.UnusedIncludes = Config::UnusedIncludesPolicy::Strict;
+  Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict;
   WithContextValue WithCfg(Config::Key, std::move(Cfg));
 
   llvm::StringMap AdditionalFiles;
Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -8,26 +8,56 @@
 
 #include "Annotations.h"
 #include "Config.h"
+#include "Diagnostics.h"
 #include "IncludeCleaner.h"
+#include "ParsedAST.h"
 #include "SourceCode.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "clang-include-cleaner/Analysis.h"
+#include "clang-include-cleaner/Types.h"
 #include "support/Context.h"
+#include "clang/AST/DeclBase.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
 namespace {
 
+using ::testing::AllOf;
 using ::testing::ElementsAre;
 using ::testing::ElementsAreArray;
 using ::testing::IsEmpty;
+using ::testing::Matcher;
 using ::testing::Pointee;
 using ::testing::UnorderedElementsAre;
 
+Matcher withFix(::testing::Matcher FixMatcher) {
+  return Field(&Diag::Fixes, ElementsAre(FixMatcher));
+}
+
+MATCHER_P2(Diag, Range, Message,
+   "Diag at " + llvm::to_string(Range) + " = [" + Message + "]") {
+  return arg.Range == Range && arg.Message == Message;
+}
+
+MATCHER_P3(Fix, Range, Replacement, Message,
+   "Fix " + llvm::to_string(Range) + " => " +
+   ::testing::PrintToString(Replacement) + " = [" + Message + "]") {
+  return arg.Message == Message && arg.Edits.size() == 1 &&
+ arg.Edits[0].range == Range && arg.Edits[0].newText == Replacement;
+}
+
 std::string guard(llvm::StringRef Code) {
   return "#pragma once\n" + Code.str();
 }
@@ -342,7 +372,8 @@
   auto AST = TU.build();
   EXPECT_THAT(computeUnusedIncludes(AST),
   ElementsAre(Pointee(writtenInclusion("";
-  EXPECT_THAT(computeUnusedIncludesExperimental(AST),
+  IncludeCleanerFindings Findings = comput

[PATCH] D143496: [clangd] Add support for missing includes analysis.

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo added a comment.

Thanks for the comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143496

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

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

[Kiran Chandramohan via Phabricator via cfe-commits]

kiranchandramohan accepted this revision.
kiranchandramohan added a comment.

LG. See one minor comment in the tests.

I would prefer having an Interface for Target Modules if that could be made to 
work. I guess this can be taken up separately after 
https://reviews.llvm.org/D144883.




Comment at: flang/test/Lower/OpenMP/omp-is-device.f90:8-17
+!DEVICE: module attributes {{{.*}}, omp.is_device = true{{.*}}}
+!HOST: module attributes {{{.*}}, omp.is_device = false{{.*}}}
+!DEVICE-FLAG-ONLY: module attributes {{{.*}}"
+!DEVICE-FLAG-ONLY-NOT: , omp.is_device = {{.*}}
+!DEVICE-FLAG-ONLY-SAME: } 
+!BBC-DEVICE: module attributes {{{.*}}, omp.is_device = true{{.*}}}
+!BBC-HOST: module attributes {{{.*}}, omp.is_device = false{{.*}}}

Any reason to have separate checks for essentially the same (e.g: DEVICE vs 
BBC-DEVICE)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144864

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

[PATCH] D141714: Fix ast print of variables with attributes

[Giuliano Belinassi via Phabricator via cfe-commits]

giulianobelinassi added a comment.

Hi, Alan. Thanks for your review again!

With regard to middle, the patch sent in D145269 
 may answer your questions. Basically 
functions like:

  int* f(void) __attribute__((unused));

would be output as

  int* __attribute__((unused)) f(void);

if SIDE_MIDDLE is specified.

The case I want to avoid is:

  int f(void) __attribute__((unused))
int i;
  { return 0; }

Which is not clear where should the attribute be applied to. A left-to-right 
parser that accepts attributes on the right side of a function declaration may 
apply this to f, but if it don't then it will be applied to i. This is why GCC 
rejects this kind of attribute output on the right side of functions.
Hence, I thought it would be better to always output variable as

  int __attribute__((unused)) var;

when possible. I also found that __declspec attributes is also accepted this 
way. But if that changes the meaning of things (I looked into generated 
assembly and could not find such case) then I think dumping it on the right 
side may be a better option, and we only have to be careful with functions 
declarations, those being dumped as.

  __attribute__((unused)) f(void) {}

-

As for changing the `__declspec` locations, I thought it was fine as it is also 
accepted by clang. But looking at MSVC documentation it says that outputing it 
at the begining is recommended so I think it may be better to always output 
__declspecs on the left side?

-

Can you explain a bit more about what warning you're seeing and under what 
condition?

Basically if you change this test in order for it to intentionally fail because 
of output mismatch, this warning is generated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141714

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

[PATCH] D145505: [AArch64][SVE] Add svboolx2_t and svboolx4_t tuple types

[Matt Devereau via Phabricator via cfe-commits]

MattDevereau created this revision.
MattDevereau added reviewers: sdesmalen, CarolineConcatto, peterwaller-arm.
Herald added subscribers: psnobl, kristof.beyls, tschuett.
Herald added a reviewer: efriedma.
Herald added a project: All.
MattDevereau requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145505

Files:
  clang/include/clang/Basic/AArch64SVEACLETypes.def
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/Type.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/test/CodeGen/svboolx2_t.cpp
  clang/test/CodeGen/svboolx4_t.cpp
  clang/test/CodeGenCXX/aarch64-mangle-sve-vectors.cpp
  clang/utils/TableGen/SveEmitter.cpp

Index: clang/utils/TableGen/SveEmitter.cpp
===
--- clang/utils/TableGen/SveEmitter.cpp
+++ clang/utils/TableGen/SveEmitter.cpp
@@ -1140,7 +1140,9 @@
   OS << "typedef __clang_svfloat16x4_t svfloat16x4_t;\n";
   OS << "typedef __clang_svfloat32x4_t svfloat32x4_t;\n";
   OS << "typedef __clang_svfloat64x4_t svfloat64x4_t;\n";
-  OS << "typedef __SVBool_t  svbool_t;\n\n";
+  OS << "typedef __SVBool_t  svbool_t;\n";
+  OS << "typedef __clang_svboolx2_t  svboolx2_t;\n";
+  OS << "typedef __clang_svboolx4_t  svboolx4_t;\n\n";
 
   OS << "typedef __clang_svbfloat16x2_t svbfloat16x2_t;\n";
   OS << "typedef __clang_svbfloat16x3_t svbfloat16x3_t;\n";
Index: clang/test/CodeGenCXX/aarch64-mangle-sve-vectors.cpp
===
--- clang/test/CodeGenCXX/aarch64-mangle-sve-vectors.cpp
+++ clang/test/CodeGenCXX/aarch64-mangle-sve-vectors.cpp
@@ -106,3 +106,7 @@
 void f47(S<__clang_svbfloat16x3_t>) {}
 // CHECK: _Z3f481SI14svbfloat16x4_tE
 void f48(S<__clang_svbfloat16x4_t>) {}
+// CHECK: _Z3f491SI10svboolx2_tE
+void f49(S<__clang_svboolx2_t>) {}
+// CHECK: _Z3f501SI10svboolx4_tE
+void f50(S<__clang_svboolx4_t>) {}
Index: clang/test/CodeGen/svboolx4_t.cpp
===
--- /dev/null
+++ clang/test/CodeGen/svboolx4_t.cpp
@@ -0,0 +1,31 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -S -emit-llvm -o - %s | FileCheck %s
+
+// CHECK-LABEL: @_Z3foo10svboolx4_t(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[ARG_ADDR:%.*]] = alloca , align 2
+// CHECK-NEXT:store  [[ARG:%.*]], ptr [[ARG_ADDR]], align 2
+// CHECK-NEXT:[[TMP0:%.*]] = load , ptr [[ARG_ADDR]], align 2
+// CHECK-NEXT:ret  [[TMP0]]
+//
+__clang_svboolx4_t foo(__clang_svboolx4_t arg) { return arg; }
+
+__clang_svboolx4_t bar();
+// CHECK-LABEL: @_Z4foo2v(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[CALL:%.*]] = call  @_Z3barv()
+// CHECK-NEXT:ret  [[CALL]]
+//
+__clang_svboolx4_t foo2() { return bar(); }
+
+__clang_svboolx4_t bar2(__clang_svboolx4_t);
+// CHECK-LABEL: @_Z4foo310svboolx4_t(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[ARG_ADDR:%.*]] = alloca , align 2
+// CHECK-NEXT:store  [[ARG:%.*]], ptr [[ARG_ADDR]], align 2
+// CHECK-NEXT:[[TMP0:%.*]] = load , ptr [[ARG_ADDR]], align 2
+// CHECK-NEXT:[[CALL:%.*]] = call  @_Z4bar210svboolx4_t( [[TMP0]])
+// CHECK-NEXT:ret  [[CALL]]
+//
+__clang_svboolx4_t foo3(__clang_svboolx4_t arg) { return bar2(arg); }
+
Index: clang/test/CodeGen/svboolx2_t.cpp
===
--- /dev/null
+++ clang/test/CodeGen/svboolx2_t.cpp
@@ -0,0 +1,31 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -S -emit-llvm -o - %s | FileCheck %s
+
+// CHECK-LABEL: @_Z3foo10svboolx2_t(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[ARG_ADDR:%.*]] = alloca , align 2
+// CHECK-NEXT:store  [[ARG:%.*]], ptr [[ARG_ADDR]], align 2
+// CHECK-NEXT:[[TMP0:%.*]] = load , ptr [[ARG_ADDR]], align 2
+// CHECK-NEXT:ret  [[TMP0]]
+//
+__clang_svboolx2_t foo(__clang_svboolx2_t arg) { return arg; }
+
+__clang_svboolx2_t bar();
+// CHECK-LABEL: @_Z4foo2v(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[CALL:%.*]] = call  @_Z3barv()
+// CHECK-NEXT:ret  [[CALL]]
+//
+__clang_svboolx2_t foo2() { return bar(); }
+
+__clang_svboolx2_t bar2(__clang_svboolx2_t);
+// CHECK-LABEL: @_Z4foo310svboolx2_t(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[ARG_ADDR:%.*]] = alloca , align 2
+// CHECK-NEXT:store  [[ARG:%.*]], ptr [[ARG_ADDR]], align 2
+// CHECK-NEXT:[[TMP0:%.*]] = load , ptr [[ARG_ADDR]], align 2
+// CHECK-NEXT:[[CALL:%.*]] = call  @_Z4bar210svboolx2_t( [[TMP0]])
+// CHECK-NEXT:ret  [[CALL]]
+//
+__clang_svboolx2_t foo3(__clang_svboolx2_t arg) { return bar2(arg); }
+
Index: clang/lib/CodeGen/CodeGenTypes.cpp
===
--- clang/lib/CodeGen/CodeGenTypes.cpp
++

[PATCH] D143496: [clangd] Add support for missing includes analysis.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks for bearing with me, let's ship it!




Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:456
+  tooling::IncludeStyle IncludeStyle;
+  auto DefaultStyle = format::getStyle(
+  format::DefaultFormatStyle, AST.tuPath(), format::DefaultFallbackStyle,

nit: this could be shorter with
```
auto FileStyle = format::getStyle(..);
if (!FileStyle) {
  elog("...");
  FileStyle = format::getLLVMStyle();
}
tooling::HeaderIncludes HeaderIncludes(AST.tuPath(), Code, 
FileStyle->IncludeStyle);
```



Comment at: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp:448
+  auto Range = MainFile.range("b");
+  auto Start = positionToOffset(MainFile.code(), Range.start);
+  EXPECT_FALSE(Start.takeError());

nit:
```
size_t Start = llvm::cantFail(positionToOffset(MainFile.code(), Range.start));
size_t End = llvm::cantFail(positionToOffset(MainFile.code(), Range.end));
```

no need for `EXPECT_FALSE(..takeError())`s as `llvm::cantFail` will fail (no 
pun intended :P), `static_cast`s are also redundant



Comment at: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp:458
+  EXPECT_THAT(Findings.MissingIncludes, ElementsAre(BInfo));
+  EXPECT_TRUE(BDecl);
+}

it'd be better to `ASSERT_TRUE(BDecl);` right after the `for loop`, as rest of 
the code will crash (and even trigger undefined behavior because we're 
dereferencing nullptr in failure case).

difference between `ASSERT_X` and `EXPECT_X` macros are, the former will stop 
execution of the particular test (hence we'll never trigger a nullptr deref 
with ASSERT_TRUE), whereas the latter just prints the failure, but doesn't 
abort the execution of test (hence helps print multiple failures at once, when 
they're non fatal).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143496

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

[PATCH] D145506: [PowerPC] Emit warn_deprecated_lax_vec_conv_all warning only for PPC

[Maryam Moghadas via Phabricator via cfe-commits]

maryammo created this revision.
Herald added subscribers: shchenz, nemanjai.
Herald added a project: All.
maryammo requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch is to isolate the lax vector conversions warning only for PPC,
the reason is that SystemZ wants different logic in terms of
vector bool compatibility.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145506

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CodeGen/SystemZ/zvector.c
  clang/test/CodeGen/SystemZ/zvector2.c


Index: clang/test/CodeGen/SystemZ/zvector2.c
===
--- clang/test/CodeGen/SystemZ/zvector2.c
+++ clang/test/CodeGen/SystemZ/zvector2.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple s390x-linux-gnu -target-cpu z14 -fzvector \
-// RUN:  -O -emit-llvm -o - -W -Wall -Werror 
-Wno-error=deprecate-lax-vec-conv-all %s | FileCheck %s
+// RUN:  -O -emit-llvm -o - -W -Wall -Werror %s | FileCheck %s
 
 volatile vector float ff, ff2;
 volatile vector bool int bi;
Index: clang/test/CodeGen/SystemZ/zvector.c
===
--- clang/test/CodeGen/SystemZ/zvector.c
+++ clang/test/CodeGen/SystemZ/zvector.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple s390x-linux-gnu -target-cpu z13 -fzvector \
-// RUN: -emit-llvm -o - -W -Wall -Werror -Wno-error=deprecate-lax-vec-conv-all 
\
+// RUN: -emit-llvm -o - -W -Wall -Werror \
 // RUN: %s | opt -S -passes=mem2reg | FileCheck %s
 
 volatile vector signed char sc, sc2;
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -1775,7 +1775,8 @@
 if (S.Context.areCompatibleVectorTypes(FromType, ToType) ||
 (S.isLaxVectorConversion(FromType, ToType) &&
  !ToType->hasAttr(attr::ArmMveStrictPolymorphism))) {
-  if (S.isLaxVectorConversion(FromType, ToType) &&
+  if (S.getASTContext().getTargetInfo().getTriple().isPPC() &&
+  S.isLaxVectorConversion(FromType, ToType) &&
   S.anyAltivecTypes(FromType, ToType) &&
   !S.Context.areCompatibleVectorTypes(FromType, ToType) &&
   !InOverloadResolution && !CStyle) {
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -9841,7 +9841,8 @@
 // The default for lax vector conversions with Altivec vectors will
 // change, so if we are converting between vector types where
 // at least one is an Altivec vector, emit a warning.
-if (anyAltivecTypes(RHSType, LHSType) &&
+if (Context.getTargetInfo().getTriple().isPPC() &&
+anyAltivecTypes(RHSType, LHSType) &&
 !Context.areCompatibleVectorTypes(RHSType, LHSType))
   Diag(RHS.get()->getExprLoc(), diag::warn_deprecated_lax_vec_conv_all)
   << RHSType << LHSType;
@@ -9858,9 +9859,10 @@
   const VectorType *VecType = RHSType->getAs();
   if (VecType && VecType->getNumElements() == 1 &&
   isLaxVectorConversion(RHSType, LHSType)) {
-if (VecType->getVectorKind() == VectorType::AltiVecVector ||
-VecType->getVectorKind() == VectorType::AltiVecBool ||
-VecType->getVectorKind() == VectorType::AltiVecPixel)
+if (Context.getTargetInfo().getTriple().isPPC() &&
+(VecType->getVectorKind() == VectorType::AltiVecVector ||
+ VecType->getVectorKind() == VectorType::AltiVecBool ||
+ VecType->getVectorKind() == VectorType::AltiVecPixel))
   Diag(RHS.get()->getExprLoc(), diag::warn_deprecated_lax_vec_conv_all)
   << RHSType << LHSType;
 ExprResult *VecExpr = &RHS;
@@ -10821,7 +10823,8 @@
   QualType OtherType = LHSVecType ? RHSType : LHSType;
   ExprResult *OtherExpr = LHSVecType ? &RHS : &LHS;
   if (isLaxVectorConversion(OtherType, VecType)) {
-if (anyAltivecTypes(RHSType, LHSType) &&
+if (Context.getTargetInfo().getTriple().isPPC() &&
+anyAltivecTypes(RHSType, LHSType) &&
 !Context.areCompatibleVectorTypes(RHSType, LHSType))
   Diag(Loc, diag::warn_deprecated_lax_vec_conv_all) << RHSType << LHSType;
 // If we're allowing lax vector conversions, only the total (data) size


Index: clang/test/CodeGen/SystemZ/zvector2.c
===
--- clang/test/CodeGen/SystemZ/zvector2.c
+++ clang/test/CodeGen/SystemZ/zvector2.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple s390x-linux-gnu -target-cpu z14 -fzvector \
-// RUN:  -O -emit-llvm -o - -W -Wall -Werror -Wno-error=deprecate-lax-vec-conv-all %s | FileCheck %s
+// RUN:  -O -emit-llvm -o - -W -Wall -Werror %s | FileCheck %s
 
 volatile vector float ff, ff2;
 volatile vector bool int bi;
Index: clang

[PATCH] D145397: [Lex] Use line markers in preprocessed assembly predefines file

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, but please add a release note about the fix when landing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145397

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

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon added a comment.

In D144864#4175257 , 
@kiranchandramohan wrote:

> LG. See one minor comment in the tests.
>
> I would prefer having an Interface for Target Modules if that could be made 
> to work. I guess this can be taken up separately after 
> https://reviews.llvm.org/D144883.

Thank you Kiran, by interface for Target Modules do you mean a new omp.module 
or this type of external interface: 
https://mlir.llvm.org/docs/Interfaces/#external-models-for-attribute-operation-and-type-interfaces

And quite possibly, @skatrak is currently looking into the review comments in 
the referenced patch (as he found similar use for the patch), so we shall see 
where the results lead to. Never opposed to improving on the infrastructure and 
this is something we will iterate on as we progress with offloading! :)




Comment at: flang/test/Lower/OpenMP/omp-is-device.f90:8-17
+!DEVICE: module attributes {{{.*}}, omp.is_device = true{{.*}}}
+!HOST: module attributes {{{.*}}, omp.is_device = false{{.*}}}
+!DEVICE-FLAG-ONLY: module attributes {{{.*}}"
+!DEVICE-FLAG-ONLY-NOT: , omp.is_device = {{.*}}
+!DEVICE-FLAG-ONLY-SAME: } 
+!BBC-DEVICE: module attributes {{{.*}}, omp.is_device = true{{.*}}}
+!BBC-HOST: module attributes {{{.*}}, omp.is_device = false{{.*}}}

kiranchandramohan wrote:
> Any reason to have separate checks for essentially the same (e.g: DEVICE vs 
> BBC-DEVICE)?
In this case to test the addition of the flag to bbc and also to the Flang 
driver, and testing the flag has the same behavior in both. There is perhaps a 
way to make the test simpler that I'm unaware of though? e.g. a way to merge 
DEVICE/BBC-DEVICE into one check


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144864

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

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

[Kiran Chandramohan via Phabricator via cfe-commits]

kiranchandramohan added a comment.

In D144864#4175332 , @agozillon wrote:

> In D144864#4175257 , 
> @kiranchandramohan wrote:
>
>> LG. See one minor comment in the tests.
>>
>> I would prefer having an Interface for Target Modules if that could be made 
>> to work. I guess this can be taken up separately after 
>> https://reviews.llvm.org/D144883.
>
> Thank you Kiran, by interface for Target Modules do you mean a new omp.module 
> or this type of external interface: 
> https://mlir.llvm.org/docs/Interfaces/#external-models-for-attribute-operation-and-type-interfaces
>
> And quite possibly, @skatrak is currently looking into the review comments in 
> the referenced patch (as he found similar use for the patch), so we shall see 
> where the results lead to. Never opposed to improving on the infrastructure 
> and this is something we will iterate on as we progress with offloading! :)

The latter one (external interface).




Comment at: flang/test/Lower/OpenMP/omp-is-device.f90:8-17
+!DEVICE: module attributes {{{.*}}, omp.is_device = true{{.*}}}
+!HOST: module attributes {{{.*}}, omp.is_device = false{{.*}}}
+!DEVICE-FLAG-ONLY: module attributes {{{.*}}"
+!DEVICE-FLAG-ONLY-NOT: , omp.is_device = {{.*}}
+!DEVICE-FLAG-ONLY-SAME: } 
+!BBC-DEVICE: module attributes {{{.*}}, omp.is_device = true{{.*}}}
+!BBC-HOST: module attributes {{{.*}}, omp.is_device = false{{.*}}}

agozillon wrote:
> kiranchandramohan wrote:
> > Any reason to have separate checks for essentially the same (e.g: DEVICE vs 
> > BBC-DEVICE)?
> In this case to test the addition of the flag to bbc and also to the Flang 
> driver, and testing the flag has the same behavior in both. There is perhaps 
> a way to make the test simpler that I'm unaware of though? e.g. a way to 
> merge DEVICE/BBC-DEVICE into one check
If DEVICE and BBC-DEVICE are checking the same things, then you only need to 
have one of them (say DEVICE) and use it in the check-prefix of both.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144864

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

[PATCH] D143496: [clangd] Add support for missing includes analysis.

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo updated this revision to Diff 503043.
VitaNuo marked 3 inline comments as done.
VitaNuo added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143496

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
  clang-tools-extra/clangd/unittests/PreambleTests.cpp
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
  clang-tools-extra/include-cleaner/lib/Analysis.cpp

Index: clang-tools-extra/include-cleaner/lib/Analysis.cpp
===
--- clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -49,8 +49,8 @@
   }
 }
 
-static std::string spellHeader(const Header &H, HeaderSearch &HS,
-   const FileEntry *Main) {
+std::string spellHeader(const Header &H, HeaderSearch &HS,
+const FileEntry *Main) {
   switch (H.kind()) {
   case Header::Physical: {
 bool IsSystem = false;
Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
===
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
@@ -73,6 +73,8 @@
 std::string fixIncludes(const AnalysisResults &Results, llvm::StringRef Code,
 const format::FormatStyle &IncludeStyle);
 
+std::string spellHeader(const Header &H, HeaderSearch &HS,
+const FileEntry *Main);
 } // namespace include_cleaner
 } // namespace clang
 
Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -665,7 +665,7 @@
 TEST(PreamblePatch, DiagnosticsToPreamble) {
   Config Cfg;
   Cfg.Diagnostics.AllowStalePreamble = true;
-  Cfg.Diagnostics.UnusedIncludes = Config::UnusedIncludesPolicy::Strict;
+  Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict;
   WithContextValue WithCfg(Config::Key, std::move(Cfg));
 
   llvm::StringMap AdditionalFiles;
Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -8,26 +8,58 @@
 
 #include "Annotations.h"
 #include "Config.h"
+#include "Diagnostics.h"
 #include "IncludeCleaner.h"
+#include "ParsedAST.h"
 #include "SourceCode.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "clang-include-cleaner/Analysis.h"
+#include "clang-include-cleaner/Types.h"
 #include "support/Context.h"
+#include "clang/AST/DeclBase.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
 namespace {
 
+using ::testing::AllOf;
 using ::testing::ElementsAre;
 using ::testing::ElementsAreArray;
 using ::testing::IsEmpty;
+using ::testing::Matcher;
 using ::testing::Pointee;
 using ::testing::UnorderedElementsAre;
 
+Matcher withFix(::testing::Matcher FixMatcher) {
+  return Field(&Diag::Fixes, ElementsAre(FixMatcher));
+}
+
+MATCHER_P2(Diag, Range, Message,
+   "Diag at " + llvm::to_string(Range) + " = [" + Message + "]") {
+  return arg.Range == Range && arg.Message == Message;
+}
+
+MATCHER_P3(Fix, Range, Replacement, Message,
+   "Fix " + llvm::to_string(Range) + " => " +
+   ::testing::PrintToString(Replacement) + " = [" + Message + "]") {
+  return arg.Message == Message && arg.Edits.size() == 1 &&
+ arg.Edits[0].range == Range && arg.Edits[0].newText == Replacement;
+}
+
 std::string guard(llvm::StringRef Code) {
   return "#pragma once\n" + Code.str();
 }
@@ -342,7 +374,8 @@
   auto AST = TU.build();
   EXPECT_THAT(computeUnusedIncludes(AST),
   ElementsAre(Pointee(writtenInclusion("";
-  EXPECT_THAT(computeUnusedIncludesExperimental(AST),

[PATCH] D145093: Add map info for dereference pointer.

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7489-7493
+  if (UO && UO->getOpcode() == UO_Deref)
+if (isa(Last->getAssociatedExpression()) ||
+isa(Last->getAssociatedExpression()) ||
+isa(Last->getAssociatedExpression()))
+  IsVarDerefAssoWithArray = true;

ABataev wrote:
> jyu2 wrote:
> > ABataev wrote:
> > > jyu2 wrote:
> > > > ABataev wrote:
> > > > > What if we have something like:
> > > > > ```
> > > > > typedef int (T)[3];
> > > > > 
> > > > > T** p;
> > > > > map(**p)
> > > > > ```
> > > > > ? Shall it work? I.e. mapping of the whole array by dereferening the 
> > > > > pointer to the array.
> > > > No, it is not work currently.  What about *(*(p+a)+b)...
> > > My question - shall it work? Mapping  (**a)[:3] should result in the same 
> > > in the same code for **a if a is
> > > ```
> > > typedef int(T)[3];
> > > T** a;
> > > ```
> > Oh, that should work.  I add additional test for that.
> > 
> > ```
> > typedef int(T)[3];
> > void bar()
> > {
> > 
> > T** a;
> > int b[2][3];
> > int (*p)[3] = b;
> > a =  &p;
> > printf("%p %p p %p\n", &a, b, *p);
> > for (int i = 0; i< 3; i++) {
> >   (**a)[1] = i;
> > }
> > #pragma omp target map((**a)[:3])
> > {
> >  (**a)[1]=5;
> >   // CHECK: 5
> >   printf("a = %d\n", (**a)[1]);
> > }
> > }
> > ```
> > 
> No, I meant different thing:
> ```
> T **a;
> .. map (**a)
> ```
Yes, that is not working currently.  That is something need to be fixed include 
*(*(a+i)+j)  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145093

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

[PATCH] D145093: Add map info for dereference pointer.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7489-7493
+  if (UO && UO->getOpcode() == UO_Deref)
+if (isa(Last->getAssociatedExpression()) ||
+isa(Last->getAssociatedExpression()) ||
+isa(Last->getAssociatedExpression()))
+  IsVarDerefAssoWithArray = true;

jyu2 wrote:
> ABataev wrote:
> > jyu2 wrote:
> > > ABataev wrote:
> > > > jyu2 wrote:
> > > > > ABataev wrote:
> > > > > > What if we have something like:
> > > > > > ```
> > > > > > typedef int (T)[3];
> > > > > > 
> > > > > > T** p;
> > > > > > map(**p)
> > > > > > ```
> > > > > > ? Shall it work? I.e. mapping of the whole array by dereferening 
> > > > > > the pointer to the array.
> > > > > No, it is not work currently.  What about *(*(p+a)+b)...
> > > > My question - shall it work? Mapping  (**a)[:3] should result in the 
> > > > same in the same code for **a if a is
> > > > ```
> > > > typedef int(T)[3];
> > > > T** a;
> > > > ```
> > > Oh, that should work.  I add additional test for that.
> > > 
> > > ```
> > > typedef int(T)[3];
> > > void bar()
> > > {
> > > 
> > > T** a;
> > > int b[2][3];
> > > int (*p)[3] = b;
> > > a =  &p;
> > > printf("%p %p p %p\n", &a, b, *p);
> > > for (int i = 0; i< 3; i++) {
> > >   (**a)[1] = i;
> > > }
> > > #pragma omp target map((**a)[:3])
> > > {
> > >  (**a)[1]=5;
> > >   // CHECK: 5
> > >   printf("a = %d\n", (**a)[1]);
> > > }
> > > }
> > > ```
> > > 
> > No, I meant different thing:
> > ```
> > T **a;
> > .. map (**a)
> > ```
> Yes, that is not working currently.  That is something need to be fixed 
> include *(*(a+i)+j)  
Not sure need to implement such complex expression, but would be good to 
support (**a) (for arrays, array sections, just a regular scalars, etc.) here 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145093

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

[PATCH] D143496: [clangd] Add support for missing includes analysis.

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo updated this revision to Diff 503045.
VitaNuo added a comment.

Rebase to main.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143496

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
  clang-tools-extra/clangd/unittests/PreambleTests.cpp
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
  clang-tools-extra/include-cleaner/lib/Analysis.cpp

Index: clang-tools-extra/include-cleaner/lib/Analysis.cpp
===
--- clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -49,8 +49,8 @@
   }
 }
 
-static std::string spellHeader(const Header &H, HeaderSearch &HS,
-   const FileEntry *Main) {
+std::string spellHeader(const Header &H, HeaderSearch &HS,
+const FileEntry *Main) {
   switch (H.kind()) {
   case Header::Physical: {
 bool IsSystem = false;
Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
===
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
@@ -73,6 +73,8 @@
 std::string fixIncludes(const AnalysisResults &Results, llvm::StringRef Code,
 const format::FormatStyle &IncludeStyle);
 
+std::string spellHeader(const Header &H, HeaderSearch &HS,
+const FileEntry *Main);
 } // namespace include_cleaner
 } // namespace clang
 
Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -665,7 +665,7 @@
 TEST(PreamblePatch, DiagnosticsToPreamble) {
   Config Cfg;
   Cfg.Diagnostics.AllowStalePreamble = true;
-  Cfg.Diagnostics.UnusedIncludes = Config::UnusedIncludesPolicy::Strict;
+  Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict;
   WithContextValue WithCfg(Config::Key, std::move(Cfg));
 
   llvm::StringMap AdditionalFiles;
Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -8,26 +8,58 @@
 
 #include "Annotations.h"
 #include "Config.h"
+#include "Diagnostics.h"
 #include "IncludeCleaner.h"
+#include "ParsedAST.h"
 #include "SourceCode.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "clang-include-cleaner/Analysis.h"
+#include "clang-include-cleaner/Types.h"
 #include "support/Context.h"
+#include "clang/AST/DeclBase.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
 namespace {
 
+using ::testing::AllOf;
 using ::testing::ElementsAre;
 using ::testing::ElementsAreArray;
 using ::testing::IsEmpty;
+using ::testing::Matcher;
 using ::testing::Pointee;
 using ::testing::UnorderedElementsAre;
 
+Matcher withFix(::testing::Matcher FixMatcher) {
+  return Field(&Diag::Fixes, ElementsAre(FixMatcher));
+}
+
+MATCHER_P2(Diag, Range, Message,
+   "Diag at " + llvm::to_string(Range) + " = [" + Message + "]") {
+  return arg.Range == Range && arg.Message == Message;
+}
+
+MATCHER_P3(Fix, Range, Replacement, Message,
+   "Fix " + llvm::to_string(Range) + " => " +
+   ::testing::PrintToString(Replacement) + " = [" + Message + "]") {
+  return arg.Message == Message && arg.Edits.size() == 1 &&
+ arg.Edits[0].range == Range && arg.Edits[0].newText == Replacement;
+}
+
 std::string guard(llvm::StringRef Code) {
   return "#pragma once\n" + Code.str();
 }
@@ -342,7 +374,8 @@
   auto AST = TU.build();
   EXPECT_THAT(computeUnusedIncludes(AST),
   ElementsAre(Pointee(writtenInclusion("";
-  EXPECT_THAT(computeUnusedIncludesExperimental(AST),
+  IncludeCleanerFindings Findings = computeInclud

[PATCH] D143587: [Docs] Multilib design

[Michael Platings via Phabricator via cfe-commits]

michaelplatings updated this revision to Diff 503046.
michaelplatings added a comment.

- Make "experimental" more obvious.
- Demonstrate using a Clang-generated flag directly in Flags


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143587

Files:
  clang/docs/Multilib.rst
  clang/docs/index.rst

Index: clang/docs/index.rst
===
--- clang/docs/index.rst
+++ clang/docs/index.rst
@@ -100,6 +100,7 @@
CodeOwners
InternalsManual
DriverInternals
+   Multilib
OffloadingDesign
PCHInternals
ItaniumMangleAbiTags
Index: clang/docs/Multilib.rst
===
--- /dev/null
+++ clang/docs/Multilib.rst
@@ -0,0 +1,324 @@
+
+Multilib
+
+
+Introduction
+
+
+This document describes how multilib is implemented in Clang.
+
+What is multilib and why might you care?
+If you're :doc:`cross compiling` then you can't use native
+system headers and libraries. To address this, you can use a combination of
+``--sysroot``, ``-isystem`` and ``-L`` options to point Clang at suitable
+directories for your target.
+However, when there are many possible directories to choose from, it's not
+necessarily obvious which one to pick.
+Multilib allows a toolchain designer to imbue the toolchain with the ability to
+pick a suitable directory automatically, based on the options the user provides
+to Clang. For example, if the user specifies
+``--target=arm-none-eabi -mcpu=cortex-m4`` the toolchain can choose a directory
+containing headers and libraries suitable for Armv7E-M, because it knows that's
+a suitable architecture for Arm Cortex-M4.
+Multilib can also choose between libraries for the same architecture based on
+other options. For example if the user specifies ``-fno-exceptions`` then a
+toolchain could select libraries built without exception support, thereby
+reducing the size of the resulting binary.
+
+Design
+==
+
+Clang supports GCC's ``-print-multi-lib`` and ``-print-multi-directory``
+options. These are described in
+`GCC Developer Options `_.
+
+There are two ways to configure multilib in Clang: hard-coded or via a
+configuration file.
+
+Hard-coded Multilib
+===
+
+The available libraries can be hard-coded in Clang. Typically this is done
+using the ``MultilibBuilder`` interface in
+``clang/include/clang/Driver/MultilibBuilder.h``.
+There are many examples of this in ``lib/Driver/ToolChains/Gnu.cpp``.
+The remainder of this document will not focus on this type of multilib.
+
+EXPERIMENTAL Multilib via configuration file
+
+
+Some Clang toolchains support loading multilib configuration from a
+``multilib.yaml`` configuration file.
+
+A ``multilib.yaml`` configuration file specifies which multilib variants are
+available, their relative location, what compilation options were used to build
+them, and the criteria by which they are selected.
+
+Multilib processing
+===
+
+Clang goes through the following steps to use multilib from a configuration
+file:
+#. Convert command line arguments to flags. Clang can accept the same
+   information via different arguments - for example,
+   ``--target=arm-none-eabi -march=armv7-m`` and
+   ``--target=armv7m-none-eabi`` are equivalent. Clang can also accept many
+   independent pieces of information within a single flag - for example
+   ``-march=armv8.1m.main+fp+mve`` specifies the architecture and two
+   extensions in a single command line argument.
+   To make it easier for the multilib system, Clang converts the command line
+   arguments into a standard set of simpler "flags". In many cases these flags
+   will look like a command line argument with the leading ``-`` stripped off,
+   but where a suitable form for the flag doesn't exist in command line
+   arguments then its form will be different. For example, an Arm architecture
+   extension is represented like ``march=+mve`` since there's no way to specify
+   it in isolation in a command line argument.
+   To see what flags are emitted for a given set of command line arguments, use
+   the ``-print-multi-selection-flags-experimental`` command line argument
+   along with the rest of the arguments you want to use.
+#. Load ``multilib.yaml`` from sysroot.
+#. Generate additional flags. ``multilib.yaml`` contains a ``FlagMap`` section,
+   which specifies how to generate additional flags based on the flags derived
+   from command line arguments. Flags are matched using regular expressions.
+   These regular expressions shall use the POSIX extended regular expression
+   syntax.
+#. Match flags against multilib variants. If the generated flags are a superset
+   of the flags specified for a multilib variant then the variant is considered

[PATCH] D145228: [clangd] Add clangd headers to install targets

[Ivan Murashko via Phabricator via cfe-commits]

ivanmurashko added a comment.

In D145228#4174829 , @sammccall wrote:

> I think this is a bit abstract though. Concretely, what API do you need here? 
> e.g. which headers do you want to include, to what end?

If we consider the bare minimum with the only goal to build outside LLVM source 
tree then we don’t need to copy all internal headers.  At the case the D145302 
 can be modified and we can end up with the 
only one header that is required to copy:  
clang-tools-extra/clangd/tool/ClangdMain.h.

For further customization one might require additional headers to be installed. 
That can be done once it’s required or via one diff that install all possible 
headers.

What do you think about it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145228

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

[PATCH] D143418: [libclang] Add API to override preamble storage path

[Igor Kushnir via Phabricator via cfe-commits]

vedgy added a comment.

I am implementing the `StorePreamblesInMemory` bool option discussed earlier.  
It would be nice to be able to modify it at any time, because it can be an 
option in an IDE's UI and requiring to restart an IDE for the option change to 
take effect is undesirable. In order to make the setter thread-safe, the option 
can be stored as `std::atomic StorePreamblesInMemory` in `class CIndexer` 
and stored/loaded with `memory_order_relaxed`. Setting this option would apply 
only to subsequent `clang_parseTranslationUnit_Impl()` calls. The option can 
also be added to `CXIndexOptions` in order to allow setting its initial value 
reliably (not worrying whether it could be used before the setter gets called 
after index construction).

Is adding both the setter and the `CXIndexOptions` member OK or would you 
prefer only one of these two?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143418

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

[clang] af682f0 - [clang] Fix single-element array initialization in constexpr

[Mariya Podchishchaeva via cfe-commits]

Author: Mariya Podchishchaeva
Date: 2023-03-07T10:57:35-05:00
New Revision: af682f0df83f3364dac7ab92f39c7209dfbce28a

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

LOG: [clang] Fix single-element array initialization in constexpr

https://reviews.llvm.org/D130791 added an improvement that in case array
element has a trivial constructor, it is evaluated once and the result is
re-used for remaining elements. Make sure the constructor is evaluated
for single-elements arrays too.

Fixes https://github.com/llvm/llvm-project/issues/60803

Reviewed By: aaron.ballman

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

Added: 
clang/test/SemaCXX/constexpr-single-element-array.cpp

Modified: 
clang/lib/AST/ExprConstant.cpp

Removed: 




diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 6b0beb1973893..26c23423b858f 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -10917,7 +10917,7 @@ bool ArrayExprEvaluator::VisitCXXConstructExpr(const 
CXXConstructExpr *E,
 for (unsigned I = OldElts; I < N; ++I)
   Value->getArrayInitializedElt(I) = Filler;
 
-  if (HasTrivialConstructor && N == FinalSize) {
+  if (HasTrivialConstructor && N == FinalSize && FinalSize != 1) {
 // If we have a trivial constructor, only evaluate it once and copy
 // the result into all the array elements.
 APValue &FirstResult = Value->getArrayInitializedElt(0);

diff  --git a/clang/test/SemaCXX/constexpr-single-element-array.cpp 
b/clang/test/SemaCXX/constexpr-single-element-array.cpp
new file mode 100644
index 0..a01b1a1c8f136
--- /dev/null
+++ b/clang/test/SemaCXX/constexpr-single-element-array.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+
+// This test makes sure that a single element array doesn't produce
+// spurious errors during constexpr evaluation.
+
+// expected-no-diagnostics
+struct Sub { int x; };
+
+struct S {
+  constexpr S() { Arr[0] = Sub{}; }
+  Sub Arr[1];
+};
+
+constexpr bool test() {
+  S s;
+  return true;
+}
+
+static_assert(test());



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

[PATCH] D145486: [clang] Fix single-element array initialization in constexpr

[Mariya Podchishchaeva via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGaf682f0df83f: [clang] Fix single-element array 
initialization in constexpr (authored by Fznamznon).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145486

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constexpr-single-element-array.cpp


Index: clang/test/SemaCXX/constexpr-single-element-array.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constexpr-single-element-array.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+
+// This test makes sure that a single element array doesn't produce
+// spurious errors during constexpr evaluation.
+
+// expected-no-diagnostics
+struct Sub { int x; };
+
+struct S {
+  constexpr S() { Arr[0] = Sub{}; }
+  Sub Arr[1];
+};
+
+constexpr bool test() {
+  S s;
+  return true;
+}
+
+static_assert(test());
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -10917,7 +10917,7 @@
 for (unsigned I = OldElts; I < N; ++I)
   Value->getArrayInitializedElt(I) = Filler;
 
-  if (HasTrivialConstructor && N == FinalSize) {
+  if (HasTrivialConstructor && N == FinalSize && FinalSize != 1) {
 // If we have a trivial constructor, only evaluate it once and copy
 // the result into all the array elements.
 APValue &FirstResult = Value->getArrayInitializedElt(0);


Index: clang/test/SemaCXX/constexpr-single-element-array.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constexpr-single-element-array.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+
+// This test makes sure that a single element array doesn't produce
+// spurious errors during constexpr evaluation.
+
+// expected-no-diagnostics
+struct Sub { int x; };
+
+struct S {
+  constexpr S() { Arr[0] = Sub{}; }
+  Sub Arr[1];
+};
+
+constexpr bool test() {
+  S s;
+  return true;
+}
+
+static_assert(test());
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -10917,7 +10917,7 @@
 for (unsigned I = OldElts; I < N; ++I)
   Value->getArrayInitializedElt(I) = Filler;
 
-  if (HasTrivialConstructor && N == FinalSize) {
+  if (HasTrivialConstructor && N == FinalSize && FinalSize != 1) {
 // If we have a trivial constructor, only evaluate it once and copy
 // the result into all the array elements.
 APValue &FirstResult = Value->getArrayInitializedElt(0);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D143496: [clangd] Add support for missing includes analysis.

[Viktoriia Bakalova 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 rG38b9fb5a129d: [clangd] Add support for missing includes 
analysis. (authored by VitaNuo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143496

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
  clang-tools-extra/clangd/unittests/PreambleTests.cpp
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
  clang-tools-extra/include-cleaner/lib/Analysis.cpp

Index: clang-tools-extra/include-cleaner/lib/Analysis.cpp
===
--- clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -49,8 +49,8 @@
   }
 }
 
-static std::string spellHeader(const Header &H, HeaderSearch &HS,
-   const FileEntry *Main) {
+std::string spellHeader(const Header &H, HeaderSearch &HS,
+const FileEntry *Main) {
   switch (H.kind()) {
   case Header::Physical: {
 bool IsSystem = false;
Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
===
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
@@ -73,6 +73,8 @@
 std::string fixIncludes(const AnalysisResults &Results, llvm::StringRef Code,
 const format::FormatStyle &IncludeStyle);
 
+std::string spellHeader(const Header &H, HeaderSearch &HS,
+const FileEntry *Main);
 } // namespace include_cleaner
 } // namespace clang
 
Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -665,7 +665,7 @@
 TEST(PreamblePatch, DiagnosticsToPreamble) {
   Config Cfg;
   Cfg.Diagnostics.AllowStalePreamble = true;
-  Cfg.Diagnostics.UnusedIncludes = Config::UnusedIncludesPolicy::Strict;
+  Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict;
   WithContextValue WithCfg(Config::Key, std::move(Cfg));
 
   llvm::StringMap AdditionalFiles;
Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -8,26 +8,58 @@
 
 #include "Annotations.h"
 #include "Config.h"
+#include "Diagnostics.h"
 #include "IncludeCleaner.h"
+#include "ParsedAST.h"
 #include "SourceCode.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "clang-include-cleaner/Analysis.h"
+#include "clang-include-cleaner/Types.h"
 #include "support/Context.h"
+#include "clang/AST/DeclBase.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
 namespace {
 
+using ::testing::AllOf;
 using ::testing::ElementsAre;
 using ::testing::ElementsAreArray;
 using ::testing::IsEmpty;
+using ::testing::Matcher;
 using ::testing::Pointee;
 using ::testing::UnorderedElementsAre;
 
+Matcher withFix(::testing::Matcher FixMatcher) {
+  return Field(&Diag::Fixes, ElementsAre(FixMatcher));
+}
+
+MATCHER_P2(Diag, Range, Message,
+   "Diag at " + llvm::to_string(Range) + " = [" + Message + "]") {
+  return arg.Range == Range && arg.Message == Message;
+}
+
+MATCHER_P3(Fix, Range, Replacement, Message,
+   "Fix " + llvm::to_string(Range) + " => " +
+   ::testing::PrintToString(Replacement) + " = [" + Message + "]") {
+  return arg.Message == Message && arg.Edits.size() == 1 &&
+ arg.Edits[0].range == Range && arg.Edits[0].newText == Replacement;
+}
+
 std::string guard(llvm::StringRef Code) {
   return "#pragma once\n" + Code.str();
 }
@@ -342,7 +374,8 @@
   auto AST = TU.build();
   EXPECT_THAT(computeUnusedIncludes(AST),
   

[clang-tools-extra] 38b9fb5 - [clangd] Add support for missing includes analysis.

[Viktoriia Bakalova via cfe-commits]

Author: Viktoriia Bakalova
Date: 2023-03-07T16:07:19Z
New Revision: 38b9fb5a129db3e086610d53b534833273c5b4d0

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

LOG: [clangd] Add support for missing includes analysis.

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

Added: 


Modified: 
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigYAML.cpp
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/IncludeCleaner.h
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
clang-tools-extra/clangd/unittests/PreambleTests.cpp
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
clang-tools-extra/include-cleaner/lib/Analysis.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Config.h 
b/clang-tools-extra/clangd/Config.h
index dffd54b01c459..ab346aab0e8ac 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -88,11 +88,12 @@ struct Config {
 bool StandardLibrary = true;
   } Index;
 
-  enum class UnusedIncludesPolicy {
-/// Diagnose unused includes.
+  enum class IncludesPolicy {
+/// Diagnose missing and unused includes.
 Strict,
 None,
-/// The same as Strict, but using the include-cleaner library.
+/// The same as Strict, but using the include-cleaner library for
+/// unused includes.
 Experiment,
   };
   /// Controls warnings and errors when parsing code.
@@ -107,11 +108,12 @@ struct Config {
   llvm::StringMap CheckOptions;
 } ClangTidy;
 
-UnusedIncludesPolicy UnusedIncludes = UnusedIncludesPolicy::None;
-
 /// Enable emitting diagnostics using stale preambles.
 bool AllowStalePreamble = false;
 
+IncludesPolicy UnusedIncludes = IncludesPolicy::None;
+IncludesPolicy MissingIncludes = IncludesPolicy::None;
+
 /// IncludeCleaner will not diagnose usages of these headers matched by
 /// these regexes.
 struct {

diff  --git a/clang-tools-extra/clangd/ConfigCompile.cpp 
b/clang-tools-extra/clangd/ConfigCompile.cpp
index 18de6e4d5c3b6..2f0ef892131ca 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -431,16 +431,16 @@ struct FragmentCompiler {
   });
 
 if (F.UnusedIncludes)
-  if (auto Val =
-  compileEnum("UnusedIncludes",
-**F.UnusedIncludes)
-  .map("Strict", Config::UnusedIncludesPolicy::Strict)
-  .map("Experiment", Config::UnusedIncludesPolicy::Experiment)
-  .map("None", Config::UnusedIncludesPolicy::None)
-  .value())
+  if (auto Val = compileEnum("UnusedIncludes",
+ **F.UnusedIncludes)
+ .map("Strict", Config::IncludesPolicy::Strict)
+ .map("Experiment", Config::IncludesPolicy::Experiment)
+ .map("None", Config::IncludesPolicy::None)
+ .value())
 Out.Apply.push_back([Val](const Params &, Config &C) {
   C.Diagnostics.UnusedIncludes = *Val;
 });
+
 if (F.AllowStalePreamble) {
   if (auto Val = F.AllowStalePreamble)
 Out.Apply.push_back([Val](const Params &, Config &C) {
@@ -448,6 +448,16 @@ struct FragmentCompiler {
 });
 }
 
+if (F.MissingIncludes)
+  if (auto Val = compileEnum("MissingIncludes",
+ **F.MissingIncludes)
+ .map("Strict", Config::IncludesPolicy::Strict)
+ .map("None", Config::IncludesPolicy::None)
+ .value())
+Out.Apply.push_back([Val](const Params &, Config &C) {
+  C.Diagnostics.MissingIncludes = *Val;
+});
+
 compile(std::move(F.Includes));
 compile(std::move(F.ClangTidy));
   }

diff  --git a/clang-tools-extra/clangd/ConfigFragment.h 
b/clang-tools-extra/clangd/ConfigFragment.h
index a5597596196fa..956e8a8599446 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -221,7 +221,7 @@ struct Fragment {
 /// This often has other advantages, such as skipping some analysis.
 std::vector> Suppress;
 
-/// Controls how clangd will correct "unnecessary #include directives.
+/// Controls how clangd will correct "unnecessary" #in

[PATCH] D145509: [HIP] Fix temporary files

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: tra, MaskRay.
Herald added a reviewer: JDevlieghere.
Herald added a project: All.
yaxunl requested review of this revision.

Currently HIP toolchain uses Driver::GetTemporaryDirectory to
create a temporary directory for some temporary files during
compilation. The temporary directories are not automatically
deleted after compilation. This slows down compilation
on Windows.

Switch to use GetTemporaryPath which only creates temporay
files which will be deleted automatically.

This change is OK for MacOS as lipo does not requires specific
file names (https://ss64.com/osx/lipo.html)


https://reviews.llvm.org/D145509

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/darwin-dsymutil.c
  clang/test/Driver/hip-link-bc-to-bc.hip
  clang/test/Driver/hip-temps-linux.hip
  clang/test/Driver/hip-temps-windows.hip

Index: clang/test/Driver/hip-temps-windows.hip
===
--- /dev/null
+++ clang/test/Driver/hip-temps-windows.hip
@@ -0,0 +1,17 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+// REQUIRES: system-windows
+
+// Check no temporary files or directores are left after compilation.
+// RUN: rm -rf %t/mytmp
+// RUN: mkdir -p %t/mytmp
+// RUN: env TMP=%t/mytmp %clang --target=x86_64-pc-windows-msvc -nogpulib -nogpuinc \
+// RUN:   --rocm-path=%S/Inputs/rocm -no-hip-rt --offload-arch=gfx1030 -v %s 2>&1 | \
+// RUN:   FileCheck -check-prefixes=CHECK %s
+// RUN: ls %t/mytmp >%t/mytmp.txt 2>&1
+// RUN: touch %t/empty.txt
+// RUN: diff %t/mytmp.txt %t/empty.txt
+
+// CHECK: -o "{{.*}}mytmp\\hip-temps-windows-gfx1030-{{.*}}.o"
+
+int main() {}
Index: clang/test/Driver/hip-temps-linux.hip
===
--- /dev/null
+++ clang/test/Driver/hip-temps-linux.hip
@@ -0,0 +1,17 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+// REQUIRES: system-linux
+
+// Check no temporary files or directores are left after compilation.
+// RUN: rm -rf %t/mytmp
+// RUN: mkdir -p %t/mytmp
+// RUN: env TMPDIR=%t/mytmp %clang --target=x86_64-linux-gnu -nogpulib -nogpuinc \
+// RUN:   --rocm-path=%S/Inputs/rocm -no-hip-rt --offload-arch=gfx1030 -v %s 2>&1 | \
+// RUN:   FileCheck -check-prefixes=CHECK %s
+// RUN: ls %t/mytmp >%t/mytmp.txt 2>&1
+// RUN: touch %t/empty.txt
+// RUN: diff %t/mytmp.txt %t/empty.txt
+
+// CHECK: -o {{.*}}/mytmp/hip-temps-linux-gfx1030-{{.*}}.o
+
+int main() {}
Index: clang/test/Driver/hip-link-bc-to-bc.hip
===
--- clang/test/Driver/hip-link-bc-to-bc.hip
+++ clang/test/Driver/hip-link-bc-to-bc.hip
@@ -11,10 +11,10 @@
 // RUN:   2>&1 | FileCheck -check-prefix=BITCODE %s
 
 // BITCODE: "{{.*}}clang-offload-bundler" "-type=bc" "-targets=host-x86_64-unknown-linux-gnu,hip-amdgcn-amd-amdhsa-gfx906" "-input={{.*}}bundle1.bc" "-output=[[B1HOST:.*\.bc]]" "-output=[[B1DEV1:.*\.bc]]" "-unbundle" "-allow-missing-bundles"
-// BITCODE: "{{.*}}clang{{.*}}" "-o" "[[B1DEV2:.*bundle1-gfx906.bc]]" "-x" "ir" "[[B1DEV1]]"
+// BITCODE: "{{.*}}clang{{.*}}" "-o" "[[B1DEV2:.*bundle1-gfx906-.*\.bc]]" "-x" "ir" "[[B1DEV1]]"
 
 // BITCODE: "{{.*}}clang-offload-bundler" "-type=bc" "-targets=host-x86_64-unknown-linux-gnu,hip-amdgcn-amd-amdhsa-gfx906" "-input={{.*}}bundle2.bc" "-output=[[B2HOST:.*\.bc]]" "-output=[[B2DEV1:.*\.bc]]" "-unbundle" "-allow-missing-bundles"
-// BITCODE: "{{.*}}clang{{.*}}" "-o" "[[B2DEV2:.*bundle2-gfx906.bc]]" "-x" "ir" "[[B2DEV1]]"
+// BITCODE: "{{.*}}clang{{.*}}" "-o" "[[B2DEV2:.*bundle2-gfx906-.*\.bc]]" "-x" "ir" "[[B2DEV1]]"
 
 // BITCODE: "{{.*}}llvm-link" "-o" "bundle1-hip-amdgcn-amd-amdhsa-gfx906.bc" "[[B1DEV2]]" "[[B2DEV2]]"
 
Index: clang/test/Driver/darwin-dsymutil.c
===
--- clang/test/Driver/darwin-dsymutil.c
+++ clang/test/Driver/darwin-dsymutil.c
@@ -48,17 +48,17 @@
 // RUN:   -arch x86_64 -arch arm64 -ccc-print-bindings %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-MULTIARCH-OUTPUT-NAME < %t %s
 //
-// CHECK-MULTIARCH-OUTPUT-NAME: "x86_64-apple-darwin11" - "darwin::Linker", inputs: ["{{.*}}{{/|\\}}darwin-dsymutil-x86_64.o"], output: "{{.*}}{{/|\\}}darwin-dsymutil-x86_64.out"
-// CHECK-MULTIARCH-OUTPUT-NAME: "arm64-apple-darwin11" - "darwin::Linker", inputs: ["{{.*}}{{/|\\}}darwin-dsymutil-arm64.o"], output: "{{.*}}{{/|\\}}darwin-dsymutil-arm64.out"
-// CHECK-MULTIARCH-OUTPUT-NAME: "arm64-apple-darwin11" - "darwin::Lipo", inputs: ["{{.*}}{{/|\\}}darwin-dsymutil-x86_64.out", "{{.*}}{{/|\\}}darwin-dsymutil-arm64.out"], output: "a.out"
+// CHECK-MULTIARCH-OUTPUT-NAME: "x86_64-apple-darwin11" - "darwin::Linker", inputs: ["{{.*}}{{/|\\}}darwin-dsymutil-x86_64{{.*}}.o"], output: "{{.*}}{{/|\\}}darwin-dsymutil-x86_64{{.*}}.out"
+// CHECK-MULTIARCH-OUTPUT-NAME: "arm64-apple-darwin11" - "darwin::Linker", inputs: ["{{.*}}{{/|\\}}darwin-dsymutil-arm64{{.*}}.o"]

[PATCH] D145486: [clang] Fix single-element array initialization in constexpr

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

Filed https://github.com/llvm/llvm-project/issues/61243 to backport.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145486

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

[PATCH] D145441: [AMDGPU] Define data layout entries for buffers

[Krzysztof Drewniak via Phabricator via cfe-commits]

krzysz00 added a comment.

@foad I was trying to avoid sending in one mega-patch that has the entire 
prototype.

As to your comments:

- Why 256-bit alignment? It's the next power of 2, and using an alignment that 
isn't a power of 2 causes an assertion failure
- Re not being allowed to use address space 8 pointers in the usual LLVM 
operations, I'd call that a target-specific rule about what address space 8 
means. I figure this is something that we can check in some sort of 
AMDGPU-specific validation
- I changed the fallback address space because the plan is that those 
intrinsics will be taking address space 8 arguments in the future


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145441

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

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski accepted this revision.
awarzynski added a comment.

In D144864#4175001 , @agozillon wrote:

> And then provided @awarzynski is happy with the current state of the patch

LGTM, thanks for the updates and for working on this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144864

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

[PATCH] D145397: [Lex] Use line markers in preprocessed assembly predefines file

[John Brawn via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG128f7dac82ea: [Lex] Use line markers in preprocessed 
assembly predefines file (authored by john.brawn).

Changed prior to commit:
  https://reviews.llvm.org/D145397?vs=502680&id=503058#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145397

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/directives_asm.S
  clang/test/Preprocessor/macro_redefined.S

Index: clang/test/Preprocessor/macro_redefined.S
===
--- /dev/null
+++ clang/test/Preprocessor/macro_redefined.S
@@ -0,0 +1,10 @@
+// RUN: %clang %s -E -DCLI_MACRO=1 2>&1 | FileCheck %s
+
+#define CLI_MACRO
+// CHECK: macro_redefined.S{{.+}}: warning: 'CLI_MACRO' macro redefined
+// CHECK: {{.+}}: note: previous definition is here
+
+#define REGULAR_MACRO
+#define REGULAR_MACRO 1
+// CHECK: macro_redefined.S{{.+}}: warning: 'REGULAR_MACRO' macro redefined
+// CHECK: macro_redefined.S{{.+}}: note: previous definition is here
Index: clang/test/Preprocessor/directives_asm.S
===
--- /dev/null
+++ clang/test/Preprocessor/directives_asm.S
@@ -0,0 +1,25 @@
+// RUN: %clang -c %s -o /dev/null 2>&1 | FileCheck %s
+
+// Check that preprocessor directives are recognised as such, but lines starting
+// with a # that aren't directives are instead treated as comments.
+
+#define MACRO .warning "This is a macro"
+MACRO
+
+// CHECK: directives_asm.S:7:9: warning: This is a macro
+
+#not a preprocessing directive
+
+// CHECK-NOT: error: invalid preprocessing directive
+
+# 100
+
+.warning "line number should not change"
+
+// CHECK: directives_asm.S:17:9: warning: line number should not change
+
+#line 100
+
+.warning "line number should change"
+
+// CHECK: directives_asm.S:101:9: warning: line number should change
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -1185,8 +1185,12 @@
 CurPPLexer->getConditionalStackDepth() > 0);
 return;
   case tok::numeric_constant:  // # 7  GNU line marker directive.
-if (getLangOpts().AsmPreprocessor)
-  break;  // # 4 is not a preprocessor directive in .S files.
+// In a .S file "# 4" may be a comment so don't treat it as a preprocessor
+// directive. However do permit it in the predefines file, as we use line
+// markers to mark the builtin macros as being in a system header.
+if (getLangOpts().AsmPreprocessor &&
+SourceMgr.getFileID(SavedHash.getLocation()) != getPredefinesFileID())
+  break;
 return HandleDigitDirective(Result);
   default:
 IdentifierInfo *II = Result.getIdentifierInfo();
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -1317,11 +1317,10 @@
   llvm::raw_string_ostream Predefines(PredefineBuffer);
   MacroBuilder Builder(Predefines);
 
-  // Emit line markers for various builtin sections of the file.  We don't do
-  // this in asm preprocessor mode, because "# 4" is not a line marker directive
-  // in this mode.
-  if (!PP.getLangOpts().AsmPreprocessor)
-Builder.append("# 1 \"\" 3");
+  // Emit line markers for various builtin sections of the file. The 3 here
+  // marks  as being a system header, which suppresses warnings when
+  // the same macro is defined multiple times.
+  Builder.append("# 1 \"\" 3");
 
   // Install things like __POWERPC__, __GNUC__, etc into the macro table.
   if (InitOpts.UsePredefines) {
@@ -1359,8 +1358,7 @@
 
   // Add on the predefines from the driver.  Wrap in a #line directive to report
   // that they come from the command line.
-  if (!PP.getLangOpts().AsmPreprocessor)
-Builder.append("# 1 \"\" 1");
+  Builder.append("# 1 \"\" 1");
 
   // Process #define's and #undef's in the order they are given.
   for (unsigned i = 0, e = InitOpts.Macros.size(); i != e; ++i) {
@@ -1372,8 +1370,7 @@
   }
 
   // Exit the command line and go back to  (2 is LC_LEAVE).
-  if (!PP.getLangOpts().AsmPreprocessor)
-Builder.append("# 1 \"\" 2");
+  Builder.append("# 1 \"\" 2");
 
   // If -imacros are specified, include them now.  These are processed before
   // any -include directives.
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -159,6 +159,9 @@
   class if that class was first introduced with a forward declaration.
 - Diagnostic notes and fix-its are now generated for ``ifunc``/``alias`` a

[clang] 128f7da - [Lex] Use line markers in preprocessed assembly predefines file

[John Brawn via cfe-commits]

Author: John Brawn
Date: 2023-03-07T16:20:43Z
New Revision: 128f7dac82ea4b996ddfd0db86edfdac4013b1da

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

LOG: [Lex] Use line markers in preprocessed assembly predefines file

GNU line marker directives are not recognised when preprocessing
assembly files, meaning they can't be used in the predefines file
meaning macros defined on the command line are reported as being
built-in.

Change this to permit line markers but only in the predefines file,
so we can correctly report command line macros as coming from the
command line.

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

Added: 
clang/test/Preprocessor/directives_asm.S
clang/test/Preprocessor/macro_redefined.S

Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Frontend/InitPreprocessor.cpp
clang/lib/Lex/PPDirectives.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 4bc2e54a85998..59c8c5c799f1a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -159,6 +159,9 @@ Improvements to Clang's diagnostics
   class if that class was first introduced with a forward declaration.
 - Diagnostic notes and fix-its are now generated for ``ifunc``/``alias`` 
attributes
   which point to functions whose names are mangled.
+- Diagnostics relating to macros on the command line of a preprocessed assembly
+  file are now reported as coming from the file  instead of
+  .
 
 Bug Fixes in This Version
 -

diff  --git a/clang/lib/Frontend/InitPreprocessor.cpp 
b/clang/lib/Frontend/InitPreprocessor.cpp
index 208c6a8db1598..ab00724af2fa9 100644
--- a/clang/lib/Frontend/InitPreprocessor.cpp
+++ b/clang/lib/Frontend/InitPreprocessor.cpp
@@ -1317,11 +1317,10 @@ void clang::InitializePreprocessor(
   llvm::raw_string_ostream Predefines(PredefineBuffer);
   MacroBuilder Builder(Predefines);
 
-  // Emit line markers for various builtin sections of the file.  We don't do
-  // this in asm preprocessor mode, because "# 4" is not a line marker 
directive
-  // in this mode.
-  if (!PP.getLangOpts().AsmPreprocessor)
-Builder.append("# 1 \"\" 3");
+  // Emit line markers for various builtin sections of the file. The 3 here
+  // marks  as being a system header, which suppresses warnings when
+  // the same macro is defined multiple times.
+  Builder.append("# 1 \"\" 3");
 
   // Install things like __POWERPC__, __GNUC__, etc into the macro table.
   if (InitOpts.UsePredefines) {
@@ -1359,8 +1358,7 @@ void clang::InitializePreprocessor(
 
   // Add on the predefines from the driver.  Wrap in a #line directive to 
report
   // that they come from the command line.
-  if (!PP.getLangOpts().AsmPreprocessor)
-Builder.append("# 1 \"\" 1");
+  Builder.append("# 1 \"\" 1");
 
   // Process #define's and #undef's in the order they are given.
   for (unsigned i = 0, e = InitOpts.Macros.size(); i != e; ++i) {
@@ -1372,8 +1370,7 @@ void clang::InitializePreprocessor(
   }
 
   // Exit the command line and go back to  (2 is LC_LEAVE).
-  if (!PP.getLangOpts().AsmPreprocessor)
-Builder.append("# 1 \"\" 2");
+  Builder.append("# 1 \"\" 2");
 
   // If -imacros are specified, include them now.  These are processed before
   // any -include directives.

diff  --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index 9e2392529ff53..bda9d0877612e 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -1185,8 +1185,12 @@ void Preprocessor::HandleDirective(Token &Result) {
 CurPPLexer->getConditionalStackDepth() > 
0);
 return;
   case tok::numeric_constant:  // # 7  GNU line marker directive.
-if (getLangOpts().AsmPreprocessor)
-  break;  // # 4 is not a preprocessor directive in .S files.
+// In a .S file "# 4" may be a comment so don't treat it as a preprocessor
+// directive. However do permit it in the predefines file, as we use line
+// markers to mark the builtin macros as being in a system header.
+if (getLangOpts().AsmPreprocessor &&
+SourceMgr.getFileID(SavedHash.getLocation()) != getPredefinesFileID())
+  break;
 return HandleDigitDirective(Result);
   default:
 IdentifierInfo *II = Result.getIdentifierInfo();

diff  --git a/clang/test/Preprocessor/directives_asm.S 
b/clang/test/Preprocessor/directives_asm.S
new file mode 100644
index 0..55e71d621e341
--- /dev/null
+++ b/clang/test/Preprocessor/directives_asm.S
@@ -0,0 +1,25 @@
+// RUN: %clang -c %s -o /dev/null 2>&1 | FileCheck %s
+
+// Check that preprocessor directives are recognised as such, but lines 
starting
+// with a # that aren't directives are instead treated as comments.
+
+#define MACRO .wa

[clang] d0db54e - [clang] Update test according to P1937

[Mariya Podchishchaeva via cfe-commits]

Author: Mariya Podchishchaeva
Date: 2023-03-07T11:30:25-05:00
New Revision: d0db54e0dd3272a044407a6394cae47c84cd3a70

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

LOG: [clang] Update test according to P1937

https://wg21.link/p1937 proposes that in unevaluated contexts, consteval
functions should not be immediately evaluated.
Clang implemented p1937 a while ago, its behavior is correct and the
test needs an update.

Reviewed By: aaron.ballman, shafik

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

Added: 


Modified: 
clang/test/CXX/expr/expr.const/p8-2a.cpp

Removed: 




diff  --git a/clang/test/CXX/expr/expr.const/p8-2a.cpp 
b/clang/test/CXX/expr/expr.const/p8-2a.cpp
index b8bee64b0aae2..c13b32a165dbd 100644
--- a/clang/test/CXX/expr/expr.const/p8-2a.cpp
+++ b/clang/test/CXX/expr/expr.const/p8-2a.cpp
@@ -2,7 +2,7 @@
 
 // expected-no-diagnostics
 
-namespace P1073R3 {
+namespace P1937R2 {
 struct N {
   constexpr N() {}
   N(N const&) = delete;
@@ -11,12 +11,22 @@ struct N {
 template constexpr void bad_assert_copyable() { T t; T t2 = t; }
 using ineffective = decltype(bad_assert_copyable());
 
-// bad_assert_copyable is not needed for constant evaluation
-// (and thus not instantiated)
 template consteval void assert_copyable() { T t; T t2 = t; }
+// Prior to P1937R2 consteval functions were evaluated even in otherwise
+// unevaluated context, now this is well-formed.
 using check = decltype(assert_copyable());
-// FIXME: this should give an error because assert_copyable is instantiated
-// (because it is needed for constant evaluation), but the attempt to copy t is
-// ill-formed.
-} // namespace P1073R3
+
+template
+__add_rvalue_reference(T) declval();
+
+constexpr auto add1(auto lhs, auto rhs) {
+return lhs + rhs;
+}
+using T = decltype(add1(declval(), declval()));
+
+consteval auto add2(auto lhs, auto rhs) {
+return lhs + rhs;
+}
+using T = decltype(add2(declval(), declval()));
+} // namespace P1937R2
 



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

[PATCH] D145362: [clang] Update test according to P1937

[Mariya Podchishchaeva via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd0db54e0dd32: [clang] Update test according to P1937 
(authored by Fznamznon).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145362

Files:
  clang/test/CXX/expr/expr.const/p8-2a.cpp


Index: clang/test/CXX/expr/expr.const/p8-2a.cpp
===
--- clang/test/CXX/expr/expr.const/p8-2a.cpp
+++ clang/test/CXX/expr/expr.const/p8-2a.cpp
@@ -2,7 +2,7 @@
 
 // expected-no-diagnostics
 
-namespace P1073R3 {
+namespace P1937R2 {
 struct N {
   constexpr N() {}
   N(N const&) = delete;
@@ -11,12 +11,22 @@
 template constexpr void bad_assert_copyable() { T t; T t2 = t; }
 using ineffective = decltype(bad_assert_copyable());
 
-// bad_assert_copyable is not needed for constant evaluation
-// (and thus not instantiated)
 template consteval void assert_copyable() { T t; T t2 = t; }
+// Prior to P1937R2 consteval functions were evaluated even in otherwise
+// unevaluated context, now this is well-formed.
 using check = decltype(assert_copyable());
-// FIXME: this should give an error because assert_copyable is instantiated
-// (because it is needed for constant evaluation), but the attempt to copy t is
-// ill-formed.
-} // namespace P1073R3
+
+template
+__add_rvalue_reference(T) declval();
+
+constexpr auto add1(auto lhs, auto rhs) {
+return lhs + rhs;
+}
+using T = decltype(add1(declval(), declval()));
+
+consteval auto add2(auto lhs, auto rhs) {
+return lhs + rhs;
+}
+using T = decltype(add2(declval(), declval()));
+} // namespace P1937R2
 


Index: clang/test/CXX/expr/expr.const/p8-2a.cpp
===
--- clang/test/CXX/expr/expr.const/p8-2a.cpp
+++ clang/test/CXX/expr/expr.const/p8-2a.cpp
@@ -2,7 +2,7 @@
 
 // expected-no-diagnostics
 
-namespace P1073R3 {
+namespace P1937R2 {
 struct N {
   constexpr N() {}
   N(N const&) = delete;
@@ -11,12 +11,22 @@
 template constexpr void bad_assert_copyable() { T t; T t2 = t; }
 using ineffective = decltype(bad_assert_copyable());
 
-// bad_assert_copyable is not needed for constant evaluation
-// (and thus not instantiated)
 template consteval void assert_copyable() { T t; T t2 = t; }
+// Prior to P1937R2 consteval functions were evaluated even in otherwise
+// unevaluated context, now this is well-formed.
 using check = decltype(assert_copyable());
-// FIXME: this should give an error because assert_copyable is instantiated
-// (because it is needed for constant evaluation), but the attempt to copy t is
-// ill-formed.
-} // namespace P1073R3
+
+template
+__add_rvalue_reference(T) declval();
+
+constexpr auto add1(auto lhs, auto rhs) {
+return lhs + rhs;
+}
+using T = decltype(add1(declval(), declval()));
+
+consteval auto add2(auto lhs, auto rhs) {
+return lhs + rhs;
+}
+using T = decltype(add2(declval(), declval()));
+} // namespace P1937R2
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] f1440bf - Driver: introduce GNU spellings to control MSVC paths

[Saleem Abdulrasool via cfe-commits]

Author: Saleem Abdulrasool
Date: 2023-03-07T08:41:35-08:00
New Revision: f1440bf6fd22ca0a5fc3594000e966201989fd48

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

LOG: Driver: introduce GNU spellings to control MSVC paths

Add a set of `-Xmicrosoft` flags to control the Windows SDK and VisualC
tools directories.  This allows control over the selection of the SDK
and tools when using the GNU driver.

Differential Revision: https://reviews.llvm.org/D145007
Reviewed By: mstorjo

Added: 


Modified: 
clang/include/clang/Driver/Options.td

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index b914b1b8f12eb..d61083f8b538a 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -7044,6 +7044,16 @@ def _SLASH_Gv : CLFlag<"Gv">,
 def _SLASH_Gregcall : CLFlag<"Gregcall">,
   HelpText<"Set __regcall as a default calling convention">;
 
+// GNU Driver aliases
+
+def : Separate<["-"], "Xmicrosoft-visualc-tools-root">, 
Alias<_SLASH_vctoolsdir>;
+def : Separate<["-"], "Xmicrosoft-visualc-tools-version">,
+Alias<_SLASH_vctoolsversion>;
+def : Separate<["-"], "Xmicrosoft-windows-sdk-root">,
+Alias<_SLASH_winsdkdir>;
+def : Separate<["-"], "Xmicrosoft-windows-sdk-version">,
+Alias<_SLASH_winsdkversion>;
+
 // Ignored:
 
 def _SLASH_analyze_ : CLIgnoredFlag<"analyze-">;



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

[PATCH] D145007: Driver: introduce GNU spellings to control MSVC paths

[Saleem Abdulrasool 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 rGf1440bf6fd22: Driver: introduce GNU spellings to control 
MSVC paths (authored by compnerd).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145007

Files:
  clang/include/clang/Driver/Options.td


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -7044,6 +7044,16 @@
 def _SLASH_Gregcall : CLFlag<"Gregcall">,
   HelpText<"Set __regcall as a default calling convention">;
 
+// GNU Driver aliases
+
+def : Separate<["-"], "Xmicrosoft-visualc-tools-root">, 
Alias<_SLASH_vctoolsdir>;
+def : Separate<["-"], "Xmicrosoft-visualc-tools-version">,
+Alias<_SLASH_vctoolsversion>;
+def : Separate<["-"], "Xmicrosoft-windows-sdk-root">,
+Alias<_SLASH_winsdkdir>;
+def : Separate<["-"], "Xmicrosoft-windows-sdk-version">,
+Alias<_SLASH_winsdkversion>;
+
 // Ignored:
 
 def _SLASH_analyze_ : CLIgnoredFlag<"analyze-">;


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -7044,6 +7044,16 @@
 def _SLASH_Gregcall : CLFlag<"Gregcall">,
   HelpText<"Set __regcall as a default calling convention">;
 
+// GNU Driver aliases
+
+def : Separate<["-"], "Xmicrosoft-visualc-tools-root">, Alias<_SLASH_vctoolsdir>;
+def : Separate<["-"], "Xmicrosoft-visualc-tools-version">,
+Alias<_SLASH_vctoolsversion>;
+def : Separate<["-"], "Xmicrosoft-windows-sdk-root">,
+Alias<_SLASH_winsdkdir>;
+def : Separate<["-"], "Xmicrosoft-windows-sdk-version">,
+Alias<_SLASH_winsdkversion>;
+
 // Ignored:
 
 def _SLASH_analyze_ : CLIgnoredFlag<"analyze-">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145514: [OPENMP]Fix PR59947: "Partially-triangular" loop collapse crashes.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev created this revision.
ABataev added reviewers: jdoerfert, Meinersbur, mikerice.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
ABataev requested review of this revision.
Herald added subscribers: openmp-commits, sstefan1.
Herald added projects: clang, OpenMP.

The indeces of the dependent loops are properly ordered, just start from
1, so need just subtract 1 to get correct loop index.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145514

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/for_non_rectangular_codegen.c
  openmp/runtime/test/worksharing/for/omp_for_non_rectangular.c

Index: openmp/runtime/test/worksharing/for/omp_for_non_rectangular.c
===
--- /dev/null
+++ openmp/runtime/test/worksharing/for/omp_for_non_rectangular.c
@@ -0,0 +1,34 @@
+// RUN: %libomp-compile-and-run
+
+#define N 10
+int arr[N][N][N];
+
+void collapsed(int mp) {
+#pragma omp for collapse(3)
+  for (int j = 0; j < mp; ++j) {
+for (int i = j; i < mp; ++i) {
+  for (int i0 = 0; i0 < N; ++i0) {
+arr[j][i][i0] = 1;
+  }
+}
+  }
+}
+
+int test(int mp) {
+  int Cnt = 0;
+  for (int j = 0; j < mp; ++j) {
+for (int i = 0; i < mp; ++i) {
+  for (int i0 = 0; i0 < N; ++i0) {
+if ((i < j && arr[j][i][i0] == 0) || (i >= j && arr[j][i][i0] == 1))
+  continue;
+++Cnt;
+  }
+}
+  }
+  return Cnt;
+}
+
+int main() {
+  collapsed(N);
+  return test(N);
+}
Index: clang/test/OpenMP/for_non_rectangular_codegen.c
===
--- /dev/null
+++ clang/test/OpenMP/for_non_rectangular_codegen.c
@@ -0,0 +1,259 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --function-signature --include-generated-funcs --replace-value-regex "__omp_offloading_[0-9a-z]+_[0-9a-z]+" "reduction_size[.].+[.]" "pl_cond[.].+[.|,]" --prefix-filecheck-ir-name _
+// RUN: %clang_cc1 -verify -fopenmp -x c -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -x c -triple x86_64-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -x c -triple x86_64-unknown-unknown -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+
+// RUN: %clang_cc1 -verify -fopenmp-simd -x c -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// RUN: %clang_cc1 -fopenmp-simd -x c -triple x86_64-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -x c -triple x86_64-unknown-unknown -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// SIMD-ONLY0-NOT: {{__kmpc|__tgt}}
+//
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+
+void collapsed(int mp) {
+#pragma omp for collapse(3)
+  for (int j = 0; j < mp; ++j) {
+for (int i = j; i < mp; ++i) {
+  for (int i0 = 0; i0 < 10; ++i0) {
+;
+  }
+}
+  }
+}
+
+#endif // HEADER
+// CHECK-LABEL: define {{[^@]+}}@collapsed
+// CHECK-SAME: (i32 noundef [[MP:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[MP_ADDR:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTOMP_IV:%.*]] = alloca i64, align 8
+// CHECK-NEXT:[[TMP:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[_TMP1:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[_TMP2:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTCAPTURE_EXPR_:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTLB_MIN:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTLB_MAX:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTMIN_LESS_MAX:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTUPPER:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTLOWER:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTCAPTURE_EXPR_3:%.*]] = alloca i64, align 8
+// CHECK-NEXT:[[J:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[I:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[I0:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[_TMP15:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTOMP_LB:%.*]] = alloca i64, align 8
+// CHECK-NEXT:[[DOTOMP_UB:%.*]] = alloca i64, align 8
+// CHECK-NEXT:[[DOTOMP_STRIDE:%.*]] = alloca i64, align 8
+// CHECK-NEXT:[[DOTOMP_IS_LAST:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[J19:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[I20:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[I021:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[TMP0:%.*]] = call i32 @__kmpc_global_thread_num(ptr @[[GLOB2:[0-9]+]])
+// CHECK-NEXT:store i32 [[MP]], ptr [[MP_ADDR]], align 4
+// CHECK-NEXT:[[TMP1:%.*]] = load i32, ptr [[MP_ADDR]], align 4
+// CHECK-NEXT:store i32 [[TMP1]], ptr [[DOTCAPTURE_EXPR_]], align 4
+// CHECK-NEXT:store i32 0, ptr [[TMP]], align 4
+// CHECK-NEXT:[[TMP2:%.*]] = load i32, ptr [[TMP]], align 4
+// CHECK-NEXT:store i32 [[TMP2]], ptr [[DOTLB_MIN]], align 4
+// CHECK-NEXT:[[TMP3:%.*]] = l

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon updated this revision to Diff 503070.
agozillon added a comment.

- [Flang][Driver] Tidy up omp-is-device.f90 test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144864

Files:
  clang/include/clang/Driver/Options.td
  flang/include/flang/Frontend/LangOptions.def
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/test/Driver/driver-help.f90
  flang/test/Lower/OpenMP/omp-is-device.f90
  flang/tools/bbc/bbc.cpp
  mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
  mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

Index: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
===
--- mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -1417,6 +1417,26 @@
   return success();
 }
 
+//===--===//
+// OpenMPDialect helper functions
+//===--===//
+
+// Set the omp.is_device attribute on the module with the specified boolean
+void OpenMPDialect::setIsDevice(Operation* module, bool isDevice) {
+  module->setAttr(
+  mlir::StringAttr::get(module->getContext(), llvm::Twine{"omp.is_device"}),
+  mlir::BoolAttr::get(module->getContext(), isDevice));
+}
+
+// Return the value of the omp.is_device attribute stored in the module if it
+// exists, otherwise return false by default
+bool OpenMPDialect::getIsDevice(Operation* module) {
+  if (Attribute isDevice = module->getAttr("omp.is_device"))
+if (isDevice.isa())
+  return isDevice.dyn_cast().getValue();
+  return false;
+}
+
 #define GET_ATTRDEF_CLASSES
 #include "mlir/Dialect/OpenMP/OpenMPOpsAttributes.cpp.inc"
 
Index: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
===
--- mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -28,6 +28,15 @@
   let cppNamespace = "::mlir::omp";
   let dependentDialects = ["::mlir::LLVM::LLVMDialect"];
   let useDefaultAttributePrinterParser = 1;
+
+  let extraClassDeclaration = [{
+// Set the omp.is_device attribute on the module with the specified boolean
+static void setIsDevice(Operation* module, bool isDevice);
+
+// Return the value of the omp.is_device attribute stored in the module if it
+// exists, otherwise return false by default
+static bool getIsDevice(Operation* module);
+  }];
 }
 
 // OmpCommon requires definition of OpenACC_Dialect.
Index: flang/tools/bbc/bbc.cpp
===
--- flang/tools/bbc/bbc.cpp
+++ flang/tools/bbc/bbc.cpp
@@ -38,6 +38,7 @@
 #include "flang/Semantics/semantics.h"
 #include "flang/Semantics/unparse-with-symbols.h"
 #include "flang/Version.inc"
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
 #include "mlir/IR/AsmState.h"
 #include "mlir/IR/BuiltinOps.h"
 #include "mlir/IR/MLIRContext.h"
@@ -122,6 +123,11 @@
 llvm::cl::desc("enable openmp"),
 llvm::cl::init(false));
 
+static llvm::cl::opt
+enableOpenMPDevice("fopenmp-is-device",
+   llvm::cl::desc("enable openmp device compilation"),
+   llvm::cl::init(false));
+
 static llvm::cl::opt enableOpenACC("fopenacc",
  llvm::cl::desc("enable openacc"),
  llvm::cl::init(false));
@@ -237,6 +243,8 @@
   kindMap, loweringOptions, {});
   burnside.lower(parseTree, semanticsContext);
   mlir::ModuleOp mlirModule = burnside.getModule();
+  if (enableOpenMP)
+mlir::omp::OpenMPDialect::setIsDevice(mlirModule, enableOpenMPDevice);
   std::error_code ec;
   std::string outputName = outputFilename;
   if (!outputName.size())
Index: flang/test/Lower/OpenMP/omp-is-device.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/omp-is-device.f90
@@ -0,0 +1,14 @@
+!RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=DEVICE
+!RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s --check-prefix=HOST
+!RUN: %flang_fc1 -emit-fir -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=DEVICE-FLAG-ONLY
+!RUN: bbc -fopenmp -fopenmp-is-device -emit-fir -o - %s | FileCheck %s --check-prefix=DEVICE
+!RUN: bbc -fopenmp -emit-fir -o - %s | FileCheck %s --check-prefix=HOST
+!RUN: bbc -fopenmp-is-device -emit-fir -o - %s | FileCheck %s --check-prefix=DEVICE-FLAG-ONLY
+
+!DEVICE: module attributes {{{.*}}, omp.is_device = true{{.*}}}
+!HOST: module attributes {{{.*}}, omp.is_device = false{{.*}}}
+!DEVICE-FLAG-ONLY: module attributes {{{.*}}"
+!DEVICE-FLAG-ONLY-NOT: , omp.is_device = {{.*}}
+!DEVICE-FLAG-ONLY-SAME: }
+subr

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon added a comment.

Cleaned up the test, thank you both for teaching me more about the compiler and 
test infrastructure! If you're happy with the test changes @kiranchandramohan 
I'll commit the changes upstream to close out the review?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144864

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

[clang] a2739f1 - [clang] Treat function parameter scope as an immediate function context

[Mariya Podchishchaeva via cfe-commits]

Author: Mariya Podchishchaeva
Date: 2023-03-07T11:46:26-05:00
New Revision: a2739f111d9795fe49109c26c2d816436f2143c3

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

LOG: [clang] Treat function parameter scope as an immediate function context

This results in expressions that appear in default function argument not
being checked for being actual constant expressions.
This aligns clang's behavior with the standard and fixes one of the
examples from https://wg21.link/P1073R3.

Reviewed By: shafik, cor3ntin

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

Added: 


Modified: 
clang/lib/Sema/SemaExpr.cpp
clang/test/CXX/expr/expr.const/p6-2a.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 56abde04eaf0d..f01329a9dfb37 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -5917,8 +5917,15 @@ bool Sema::CheckCXXDefaultArgExpr(SourceLocation 
CallLoc, FunctionDecl *FD,
 assert(!InitWithCleanup->getNumObjects() &&
"default argument expression has capturing blocks?");
   }
+  // C++ [expr.const]p15.1:
+  //   An expression or conversion is in an immediate function context if it is
+  //   potentially evaluated and [...] its innermost enclosing non-block scope
+  //   is a function parameter scope of an immediate function.
   EnterExpressionEvaluationContext EvalContext(
-  *this, ExpressionEvaluationContext::PotentiallyEvaluated, Param);
+  *this,
+  FD->isConsteval() ? ExpressionEvaluationContext::ImmediateFunctionContext
+: ExpressionEvaluationContext::PotentiallyEvaluated,
+  Param);
   ExprEvalContexts.back().IsCurrentlyCheckingDefaultArgumentOrInitializer =
   SkipImmediateInvocations;
   MarkDeclarationsReferencedInExpr(Init, /*SkipLocalVariables*/ true);
@@ -6005,8 +6012,16 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation 
CallLoc,
 // Mark that we are replacing a default argument first.
 // If we are instantiating a template we won't have to
 // retransform immediate calls.
+// C++ [expr.const]p15.1:
+//   An expression or conversion is in an immediate function context if it
+//   is potentially evaluated and [...] its innermost enclosing non-block
+//   scope is a function parameter scope of an immediate function.
 EnterExpressionEvaluationContext EvalContext(
-*this, ExpressionEvaluationContext::PotentiallyEvaluated, Param);
+*this,
+FD->isConsteval()
+? ExpressionEvaluationContext::ImmediateFunctionContext
+: ExpressionEvaluationContext::PotentiallyEvaluated,
+Param);
 
 if (Param->hasUninstantiatedDefaultArg()) {
   if (InstantiateDefaultArgument(CallLoc, FD, Param))

diff  --git a/clang/test/CXX/expr/expr.const/p6-2a.cpp 
b/clang/test/CXX/expr/expr.const/p6-2a.cpp
index 2ef067c549003..a937474d53b22 100644
--- a/clang/test/CXX/expr/expr.const/p6-2a.cpp
+++ b/clang/test/CXX/expr/expr.const/p6-2a.cpp
@@ -43,14 +43,12 @@ struct Temporary {
 constexpr Temporary t = {3}; // expected-error {{must have constant 
destruction}} expected-note {{created here}} expected-note {{in call}}
 
 namespace P1073R3 {
-consteval int f() { return 42; } // expected-note 3 {{declared here}}
+consteval int f() { return 42; } // expected-note 2 {{declared here}}
 consteval auto g() { return f; }
-// FIXME: there should be no diagnostics associated with either h() or r.
-consteval int h(int (*p)() = g()) { return p(); } // expected-error {{call to 
consteval function 'P1073R3::g' is not a constant expression}} \
- expected-note {{declared 
here}} \
- expected-note {{pointer 
to a consteval declaration is not a constant expression}}
-constexpr int r = h();   // expected-note {{in the default initalizer of 'p'}}
+consteval int h(int (*p)() = g()) { return p(); }
+constexpr int r = h();
 constexpr auto e = g();  // expected-error {{call to consteval function 
'P1073R3::g' is not a constant expression}} \
 expected-error {{constexpr variable 'e' must be 
initialized by a constant expression}} \
 expected-note 2 {{pointer to a consteval 
declaration is not a constant expression}}
+static_assert(r == 42);
 } // namespace P1073R3



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

[PATCH] D145251: [clang] Treat function parameter scope as an immediate function context

[Mariya Podchishchaeva via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa2739f111d97: [clang] Treat function parameter scope as an 
immediate function context (authored by Fznamznon).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145251

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CXX/expr/expr.const/p6-2a.cpp


Index: clang/test/CXX/expr/expr.const/p6-2a.cpp
===
--- clang/test/CXX/expr/expr.const/p6-2a.cpp
+++ clang/test/CXX/expr/expr.const/p6-2a.cpp
@@ -43,14 +43,12 @@
 constexpr Temporary t = {3}; // expected-error {{must have constant 
destruction}} expected-note {{created here}} expected-note {{in call}}
 
 namespace P1073R3 {
-consteval int f() { return 42; } // expected-note 3 {{declared here}}
+consteval int f() { return 42; } // expected-note 2 {{declared here}}
 consteval auto g() { return f; }
-// FIXME: there should be no diagnostics associated with either h() or r.
-consteval int h(int (*p)() = g()) { return p(); } // expected-error {{call to 
consteval function 'P1073R3::g' is not a constant expression}} \
- expected-note {{declared 
here}} \
- expected-note {{pointer 
to a consteval declaration is not a constant expression}}
-constexpr int r = h();   // expected-note {{in the default initalizer of 'p'}}
+consteval int h(int (*p)() = g()) { return p(); }
+constexpr int r = h();
 constexpr auto e = g();  // expected-error {{call to consteval function 
'P1073R3::g' is not a constant expression}} \
 expected-error {{constexpr variable 'e' must be 
initialized by a constant expression}} \
 expected-note 2 {{pointer to a consteval 
declaration is not a constant expression}}
+static_assert(r == 42);
 } // namespace P1073R3
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -5917,8 +5917,15 @@
 assert(!InitWithCleanup->getNumObjects() &&
"default argument expression has capturing blocks?");
   }
+  // C++ [expr.const]p15.1:
+  //   An expression or conversion is in an immediate function context if it is
+  //   potentially evaluated and [...] its innermost enclosing non-block scope
+  //   is a function parameter scope of an immediate function.
   EnterExpressionEvaluationContext EvalContext(
-  *this, ExpressionEvaluationContext::PotentiallyEvaluated, Param);
+  *this,
+  FD->isConsteval() ? ExpressionEvaluationContext::ImmediateFunctionContext
+: ExpressionEvaluationContext::PotentiallyEvaluated,
+  Param);
   ExprEvalContexts.back().IsCurrentlyCheckingDefaultArgumentOrInitializer =
   SkipImmediateInvocations;
   MarkDeclarationsReferencedInExpr(Init, /*SkipLocalVariables*/ true);
@@ -6005,8 +6012,16 @@
 // Mark that we are replacing a default argument first.
 // If we are instantiating a template we won't have to
 // retransform immediate calls.
+// C++ [expr.const]p15.1:
+//   An expression or conversion is in an immediate function context if it
+//   is potentially evaluated and [...] its innermost enclosing non-block
+//   scope is a function parameter scope of an immediate function.
 EnterExpressionEvaluationContext EvalContext(
-*this, ExpressionEvaluationContext::PotentiallyEvaluated, Param);
+*this,
+FD->isConsteval()
+? ExpressionEvaluationContext::ImmediateFunctionContext
+: ExpressionEvaluationContext::PotentiallyEvaluated,
+Param);
 
 if (Param->hasUninstantiatedDefaultArg()) {
   if (InstantiateDefaultArgument(CallLoc, FD, Param))


Index: clang/test/CXX/expr/expr.const/p6-2a.cpp
===
--- clang/test/CXX/expr/expr.const/p6-2a.cpp
+++ clang/test/CXX/expr/expr.const/p6-2a.cpp
@@ -43,14 +43,12 @@
 constexpr Temporary t = {3}; // expected-error {{must have constant destruction}} expected-note {{created here}} expected-note {{in call}}
 
 namespace P1073R3 {
-consteval int f() { return 42; } // expected-note 3 {{declared here}}
+consteval int f() { return 42; } // expected-note 2 {{declared here}}
 consteval auto g() { return f; }
-// FIXME: there should be no diagnostics associated with either h() or r.
-consteval int h(int (*p)() = g()) { return p(); } // expected-error {{call to consteval function 'P1073R3::g' is not a constant expression}} \
- expected-note {{declared here}} \
- expected-note {{pointer to a consteval declaration is not a constant expression}}
-constexpr int r = h();   // expected-note {{in the default in

[PATCH] D145151: clang: Handle MatrixType in hasFloatingRepresentation

[Florian Hahn via Phabricator via cfe-commits]

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

LGTM, thanks!


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

https://reviews.llvm.org/D145151

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