[PATCH] D113840: [Driver][Android] Remove unneeded isNoExecStackDefault

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: danalbert, pirama.
Herald added a subscriber: danielkiss.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

ld.lld used by Android ignores .note.GNU-stack and defaults to noexecstack,
so the `-z noexecstack` linker option is unneeded.

The `--noexecstack` assembler option is unneeded because AsmPrinter.cpp
prints `.section .note.GNU-stack,"",@progbits` (when `llvm.init.trampoline` is 
unused),
so the assembler won't synthesize an executable .note.GNU-stack.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113840

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/Linux.h
  clang/test/Driver/integrated-as.c
  clang/test/Driver/linux-as.c
  clang/test/Driver/linux-ld.c

Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -989,15 +989,6 @@
 // CHECK-ANDROID-HASH-STYLE-M: "{{.*}}ld{{(.exe)?}}"
 // CHECK-ANDROID-HASH-STYLE-M: "--hash-style=gnu"
 
-// RUN: %clang %s -### -o %t.o 2>&1 \
-// RUN: --target=armv7-linux-android21 \
-// RUN:   | FileCheck --check-prefix=CHECK-ANDROID-NOEXECSTACK %s
-// CHECK-ANDROID-NOEXECSTACK: "{{.*}}ld{{(.exe)?}}"
-// CHECK-ANDROID-NOEXECSTACK: "-z" "noexecstack"
-// CHECK-ANDROID-NOEXECSTACK-NOT: "-z" "execstack"
-// CHECK-ANDROID-NOEXECSTACK-NOT: "-z,execstack"
-// CHECK-ANDROID-NOEXECSTACK-NOT: "-zexecstack"
-
 // RUN: %clang %s -### -o %t.o 2>&1 \
 // RUN: --target=armv7-linux-android21 \
 // RUN:   | FileCheck --check-prefix=CHECK-ANDROID-WARN-SHARED-TEXTREL %s
Index: clang/test/Driver/linux-as.c
===
--- clang/test/Driver/linux-as.c
+++ clang/test/Driver/linux-as.c
@@ -108,12 +108,12 @@
 // RUN: %clang -target arm-linux-androideabi -### \
 // RUN:   -no-integrated-as -c %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ARM-ANDROID %s
-// CHECK-ARM-ANDROID: as{{(.exe)?}}" "--noexecstack" "-EL" "-mfloat-abi=soft"
+// CHECK-ARM-ANDROID: as{{(.exe)?}}" "-EL" "-mfloat-abi=soft"
 //
 // RUN: %clang -target arm-linux-androideabi -march=armv7-a -### \
 // RUN:   -no-integrated-as -c %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ARM-ANDROID-SOFTFP %s
-// CHECK-ARM-ANDROID-SOFTFP: as{{(.exe)?}}" "--noexecstack" "-EL" "-mfloat-abi=softfp" "-march=armv7-a"
+// CHECK-ARM-ANDROID-SOFTFP: as{{(.exe)?}}" "-EL" "-mfloat-abi=softfp" "-march=armv7-a"
 //
 // RUN: %clang -target arm-linux-eabi -mhard-float -### \
 // RUN:   -no-integrated-as -c %s 2>&1 \
Index: clang/test/Driver/integrated-as.c
===
--- clang/test/Driver/integrated-as.c
+++ clang/test/Driver/integrated-as.c
@@ -14,8 +14,3 @@
 // NOFIAS: -cc1
 // NOFIAS: "-fno-verbose-asm"
 // NOFIAS: -no-integrated-as
-
-// RUN: %clang -target arm-linux-androideabi -### \
-// RUN:   -integrated-as -c %s 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHECK-ARM-ANDROID %s
-// CHECK-ARM-ANDROID: "-mnoexecstack"
Index: clang/lib/Driver/ToolChains/Linux.h
===
--- clang/lib/Driver/ToolChains/Linux.h
+++ clang/lib/Driver/ToolChains/Linux.h
@@ -44,7 +44,6 @@
   bool
   IsAArch64OutlineAtomicsDefault(const llvm::opt::ArgList ) const override;
   bool isPIEDefault(const llvm::opt::ArgList ) const override;
-  bool isNoExecStackDefault() const override;
   bool IsMathErrnoDefault() const override;
   SanitizerMask getSupportedSanitizers() const override;
   void addProfileRTLibs(const llvm::opt::ArgList ,
Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -680,10 +680,6 @@
   return true;
 }
 
-bool Linux::isNoExecStackDefault() const {
-return getTriple().isAndroid();
-}
-
 bool Linux::IsMathErrnoDefault() const {
   if (getTriple().isAndroid())
 return false;
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -429,11 +429,6 @@
 CmdArgs.push_back("text");
   }
 
-  if (ToolChain.isNoExecStackDefault()) {
-CmdArgs.push_back("-z");
-CmdArgs.push_back("noexecstack");
-  }
-
   if (Args.hasArg(options::OPT_rdynamic))
 CmdArgs.push_back("-export-dynamic");
 
@@ -707,10 +702,6 @@
 }
   }
 
-  if (getToolChain().isNoExecStackDefault()) {
-  CmdArgs.push_back("--noexecstack");
-  }
-
   switch (getToolChain().getArch()) {
   default:
 break;
Index: clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D108479#3129571 , @jrtc27 wrote:

> For CHERI there's the added complication that descriptors and trampolines can 
> exist for security reasons when crossing security domains, and you absolutely 
> should not let one compartment get pointers to the entry point of another 
> compartment's function. You can hand it out if sealed or the permissions are 
> cleared, as then you can't really do anything with it other than look at the 
> integer address, but that seems a bit odd.

That would be consistent with getting an unsigned pointer under pointer 
authentication or an address with the THUMB bit potentially stripped: it's just 
a raw address that you can't safely call.  In any case, I suspect it would be 
fine for us to say that we just don't support this builtin when the target is 
using weird function pointers unless we have some way to bypass the special 
treatment in LLVM.

I agree that "symbol" address is probably the wrong name.  Maybe 
`__builtin_function_start` or something like that?  But before we go much 
further on this, we should get confirmation from Peter that we're targeting the 
right design.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108479

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

[PATCH] D111971: [clang] Allocate 2 bits to store the constexpr specifier kind when serializing

[Nathan Ridge via Phabricator via cfe-commits]

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

Sorry for the delay here.

@adamcz could you kindly have another look to make sure the test changes look 
ok?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111971

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

[PATCH] D111971: [clang] Allocate 2 bits to store the constexpr specifier kind when serializing

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 387057.
nridge added a comment.

Reworked test to use the framework in clang/test/AST


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111971

Files:
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/AST/ast-dump-constant-expr.cpp

Index: clang/test/AST/ast-dump-constant-expr.cpp
===
--- clang/test/AST/ast-dump-constant-expr.cpp
+++ clang/test/AST/ast-dump-constant-expr.cpp
@@ -6,6 +6,7 @@
 // RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-unknown -emit-pch -o %t %s
 // RUN: %clang_cc1 -x c++ -std=c++20 -triple x86_64-unknown-unknown -include-pch %t \
 // RUN: -ast-dump-all -ast-dump-filter Test /dev/null \
+// RUN: | sed -e "s/ //" -e "s/ imported//" \
 // RUN: | FileCheck --strict-whitespace --match-full-lines %s
 
 // FIXME: ASTRecordReader::readAPValue and ASTRecordWriter::AddAPValue
@@ -39,47 +40,54 @@
 // FIXME: consteval U test_Union() { return U(); }
 // FIXME: consteval SU test_SU() { return SU(); }
 
-void Test() {
-  (void) test_Int();
-  (void) test_Float();
-  (void) test_ComplexInt();
-  (void) test_ComplexFloat();
-  (void) test_Int128();
-  //(void) test_Array();
-  //(void) test_Struct();
-  //(void) test_Union();
-  //(void) test_SU();
-}
+struct Test {
+  void test() {
+(void)test_Int();
+(void)test_Float();
+(void)test_ComplexInt();
+(void)test_ComplexFloat();
+(void)test_Int128();
+//(void) test_Array();
+//(void) test_Struct();
+//(void) test_Union();
+//(void) test_SU();
+  }
+
+  consteval void consteval_method() {}
+};
+
 // CHECK:Dumping Test:
-// CHECK-NEXT:FunctionDecl {{.*}} <{{.*}}ast-dump-constant-expr.cpp:42:1, line:52:1> line:42:6{{( imported)?}} Test 'void ()'
-// CHECK-NEXT:`-CompoundStmt {{.*}} 
-// CHECK-NEXT:  |-CStyleCastExpr {{.*}}  'void' 
-// CHECK-NEXT:  | `-ConstantExpr {{.*}}  'int'
-// CHECK-NEXT:  |   |-value: Int 42
-// CHECK-NEXT:  |   `-CallExpr {{.*}}  'int'
-// CHECK-NEXT:  | `-ImplicitCastExpr {{.*}}  'int (*)()' 
-// CHECK-NEXT:  |   `-DeclRefExpr {{.*}}  'int ()' lvalue Function {{.*}} 'test_Int' 'int ()'
-// CHECK-NEXT:  |-CStyleCastExpr {{.*}}  'void' 
-// CHECK-NEXT:  | `-ConstantExpr {{.*}}  'float'
-// CHECK-NEXT:  |   |-value: Float 1.00e+00
-// CHECK-NEXT:  |   `-CallExpr {{.*}}  'float'
-// CHECK-NEXT:  | `-ImplicitCastExpr {{.*}}  'float (*)()' 
-// CHECK-NEXT:  |   `-DeclRefExpr {{.*}}  'float ()' lvalue Function {{.*}} 'test_Float' 'float ()'
-// CHECK-NEXT:  |-CStyleCastExpr {{.*}}  'void' 
-// CHECK-NEXT:  | `-ConstantExpr {{.*}}  '_Complex int'
-// CHECK-NEXT:  |   |-value: ComplexInt 1 + 2i
-// CHECK-NEXT:  |   `-CallExpr {{.*}}  '_Complex int'
-// CHECK-NEXT:  | `-ImplicitCastExpr {{.*}}  '_Complex int (*)()' 
-// CHECK-NEXT:  |   `-DeclRefExpr {{.*}}  '_Complex int ()' lvalue Function {{.*}} 'test_ComplexInt' '_Complex int ()'
-// CHECK-NEXT:  |-CStyleCastExpr {{.*}}  'void' 
-// CHECK-NEXT:  | `-ConstantExpr {{.*}}  '_Complex float'
-// CHECK-NEXT:  |   |-value: ComplexFloat 1.20e+00 + 3.40e+00i
-// CHECK-NEXT:  |   `-CallExpr {{.*}}  '_Complex float'
-// CHECK-NEXT:  | `-ImplicitCastExpr {{.*}}  '_Complex float (*)()' 
-// CHECK-NEXT:  |   `-DeclRefExpr {{.*}}  '_Complex float ()' lvalue Function {{.*}} 'test_ComplexFloat' '_Complex float ()'
-// CHECK-NEXT:  `-CStyleCastExpr {{.*}}  'void' 
-// CHECK-NEXT:`-ConstantExpr {{.*}}  '__int128'
-// CHECK-NEXT:  |-value: Int 18446744073709551616
-// CHECK-NEXT:  `-CallExpr {{.*}}  '__int128'
-// CHECK-NEXT:`-ImplicitCastExpr {{.*}}  '__int128 (*)()' 
-// CHECK-NEXT:  `-DeclRefExpr {{.*}}  '__int128 ()' lvalue Function {{.*}} 'test_Int128' '__int128 ()'
+// CHECK-NEXT:CXXRecordDecl {{.*}} <{{.*}}ast-dump-constant-expr.cpp:43:1, line:57:1> line:43:8 struct Test definition
+// CHECK:|-CXXMethodDecl {{.*}}  line:44:8 test 'void ()'
+// CHECK-NEXT:| `-CompoundStmt {{.*}} 
+// CHECK-NEXT:|   |-CStyleCastExpr {{.*}}  'void' 
+// CHECK-NEXT:|   | `-ConstantExpr {{.*}}  'int'
+// CHECK-NEXT:|   |   |-value: Int 42
+// CHECK-NEXT:|   |   `-CallExpr {{.*}}  'int'
+// CHECK-NEXT:|   | `-ImplicitCastExpr {{.*}}  'int (*)()' 
+// CHECK-NEXT:|   |   `-DeclRefExpr {{.*}}  'int ()' lvalue Function {{.*}} 'test_Int' 'int ()'
+// CHECK-NEXT:|   |-CStyleCastExpr {{.*}}  'void' 
+// CHECK-NEXT:|   | `-ConstantExpr {{.*}}  'float'
+// CHECK-NEXT:|   |   |-value: Float 1.00e+00
+// CHECK-NEXT:|   |   `-CallExpr {{.*}}  'float'
+// CHECK-NEXT:|   | `-ImplicitCastExpr {{.*}}  'float (*)()' 
+// CHECK-NEXT:|   |   `-DeclRefExpr {{.*}}  'float ()' lvalue Function {{.*}} 'test_Float' 'float ()'
+// CHECK-NEXT:|   |-CStyleCastExpr {{.*}}  'void' 
+// CHECK-NEXT:|   | `-ConstantExpr {{.*}}  '_Complex int'
+// CHECK-NEXT:|   |   |-value: ComplexInt 1 + 2i
+// CHECK-NEXT:|   |   `-CallExpr {{.*}}  

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

If it's bypassing the descriptors then __builtin_symbol_address is the wrong 
name (and a bit ambiguous). As far as dlsym is concerned, the symbol is the 
descriptor, but when you get down to the ELF representation itself that's not 
always true. For PPC64 ELFv1, the ELF symbol is the descriptor, and the entry 
point has a different name. For PA-RISC and Itanium, the ELF symbol is the 
entry point, and you request the descriptor rather than the entry point by 
using a different relocation to the normal data pointer one (well, Itanium has 
a whole set of them, you have {32,64} x {LSB,MSB} plus a 64I one for putting 
into an X format instruction's immediate, and GP-relative GOT-indirect 
(`@ltoff`) versions of all those, plus a bonus 22-bit immediate one for that).

For CHERI there's the added complication that descriptors and trampolines can 
exist for security reasons when crossing security domains, and you absolutely 
should not let one compartment get pointers to the entry point of another 
compartment's function. You can hand it out if sealed or the permissions are 
cleared, as then you can't really do anything with it other than look at the 
integer address, but that seems a bit odd.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108479

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

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I don't know for certain, but I would guess that the kernel wants to get the 
address of the first instruction in the function for the purposes of some sort 
of later PC-based table lookup, which means that yes, it probably *does* need 
to bypass descriptors on CHERI / Itanium / whatever else.

What we're trying to do here is *avoid* a can of worms by clearly understanding 
what the desired semantics are, rather than adding a builtin with the semantics 
of "ignore exactly the one complication that we ran into first".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108479

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

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

In D108479#3110003 , @rjmccall wrote:

> `std::addressof()` certainly ought to return a signed pointer 
> under ptrauth, so if your goal here is to get a completely unadorned symbol 
> address, I think you do need a different builtin, and it probably ought to 
> return a `void*` to emphasize that it shouldn't be used as a normal value.  
> Maybe it should even be semantically restricted to require a constant decl 
> reference as its operand?  Related and perhaps illuminating question: if it 
> were implementable, would you also want to force the suppression of 
> lazy-binding thunks and/or decorations like the THUMB bit?

Similarly, should it point to the descriptor or the entry point for ABIs with 
function descriptors? Personally I think trying to generalise this builtin just 
opens a huge can of worms when you look at other architectures and ABIs (and 
what it does can have implications for CHERI/Morello where we have various 
weird experimental ABIs), so you may just be best off having the original 
specialised builtin with very clear semantics.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108479

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

[PATCH] D104830: AST: Create __va_list in the std namespace even in C.

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

I also note that 32-bit Arm uses the same namespacing, and has CFI-icall 
support (at least, it's enabled, I don't know if it's actually expected to 
work) but did not have this change made to it (though probably best not to do 
so until the DWARF issue is resolved)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104830

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

[PATCH] D104830: AST: Create __va_list in the std namespace even in C.

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

Or you can still do it in the frontend, just make sure that the DWARF emitted 
by the frontend doesn't have the DINamespace wrapper when not compiling C++.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104830

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

[PATCH] D99466: Fix PR48889: assertion failure for void() in flattened ternary expression

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert updated this revision to Diff 387056.
aaronpuchert marked an inline comment as done.
aaronpuchert added a comment.

Also fix code generation for `void{}`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99466

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGenCXX/value-init.cpp


Index: clang/test/CodeGenCXX/value-init.cpp
===
--- clang/test/CodeGenCXX/value-init.cpp
+++ clang/test/CodeGenCXX/value-init.cpp
@@ -338,4 +338,15 @@
   struct optional : optional_data_dtor_base, optional_assign_base {};
   optional f(optional a) { return {optional(a)}; }
 }
+
+namespace PR48889 {
+  // Just make sure we don't hit an assertion, the IR is otherwise unchanged.
+  constexpr void f() {}
+  void paren_list(bool b) {
+return b ? f() : void();
+  }
+  void braced_list(bool b) {
+return b ? f() : void{};
+  }
+}
 #endif
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -1523,6 +1523,10 @@
 }
 
 Value *ScalarExprEmitter::EmitNullValue(QualType Ty) {
+  // [expr.type.conv]: if the type is cv void and the initializer is () or {},
+  // the expression is a prvalue of type void that performs no initialization.
+  if (Ty->isVoidType())
+return nullptr;
   return CGF.EmitFromMemory(CGF.CGM.EmitNullConstant(Ty), Ty);
 }
 


Index: clang/test/CodeGenCXX/value-init.cpp
===
--- clang/test/CodeGenCXX/value-init.cpp
+++ clang/test/CodeGenCXX/value-init.cpp
@@ -338,4 +338,15 @@
   struct optional : optional_data_dtor_base, optional_assign_base {};
   optional f(optional a) { return {optional(a)}; }
 }
+
+namespace PR48889 {
+  // Just make sure we don't hit an assertion, the IR is otherwise unchanged.
+  constexpr void f() {}
+  void paren_list(bool b) {
+return b ? f() : void();
+  }
+  void braced_list(bool b) {
+return b ? f() : void{};
+  }
+}
 #endif
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -1523,6 +1523,10 @@
 }
 
 Value *ScalarExprEmitter::EmitNullValue(QualType Ty) {
+  // [expr.type.conv]: if the type is cv void and the initializer is () or {},
+  // the expression is a prvalue of type void that performs no initialization.
+  if (Ty->isVoidType())
+return nullptr;
   return CGF.EmitFromMemory(CGF.CGM.EmitNullConstant(Ty), Ty);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104830: AST: Create __va_list in the std namespace even in C.

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

This results in the DWARF type info containing DW_TAG_namespace for C, which 
breaks DTrace's CTF (as the name implies, it is for C, not C++). This does not 
seem correct to me. If your mangling for va_list is broken then you should 
special case that in the CFI mangler as an ABI quirk, IMO, not change the DWARF 
for C to contain C++ things.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104830

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

[PATCH] D113838: Sema: Don't erroneously reject `void{}`

[Aaron Puchert via Phabricator via cfe-commits]

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

This is explicitly allowed via [expr.type.conv], if the initialization
list is empty.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113838

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaInit.cpp
  clang/test/CXX/expr/expr.post/expr.type.conv/p2.cpp
  clang/test/SemaCXX/attr-annotate.cpp
  clang/test/SemaCXX/cxx2a-explicit-bool.cpp


Index: clang/test/SemaCXX/cxx2a-explicit-bool.cpp
===
--- clang/test/SemaCXX/cxx2a-explicit-bool.cpp
+++ clang/test/SemaCXX/cxx2a-explicit-bool.cpp
@@ -73,8 +73,8 @@
 
 template  struct E {
   // expected-note@-1+ {{candidate constructor}}
-  explicit((T{}, false))
-  // expected-error@-1 {{illegal initializer type 'void'}}
+  explicit(((T&&)T{}, false))
+  // expected-error@-1 {{cannot form a reference to 'void'}}
   E(int);
 };
 
Index: clang/test/SemaCXX/attr-annotate.cpp
===
--- clang/test/SemaCXX/attr-annotate.cpp
+++ clang/test/SemaCXX/attr-annotate.cpp
@@ -42,8 +42,8 @@
 
   template
   struct B {
-[[clang::annotate("test", ((void)T{}, 9))]] void t() {}
-// expected-error@-1 {{illegal initializer type 'void'}}
+[[clang::annotate("test", ((void)T{1}, 9))]] void t() {}
+// expected-error@-1 {{initializer list for 'void' must be empty}}
   };
   B b;
   B b1;
Index: clang/test/CXX/expr/expr.post/expr.type.conv/p2.cpp
===
--- /dev/null
+++ clang/test/CXX/expr/expr.post/expr.type.conv/p2.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+void paren_list() {
+  return void();
+}
+
+void brace_list() {
+  return void{};
+}
+
+void brace_list_nonempty() {
+  return void{1}; // expected-error {{initializer list for 'void' must be 
empty}}
+}
+
+void brace_list_nested_nonempty() {
+  return void{{}}; // expected-error {{initializer list for 'void' must be 
empty}}
+}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -1308,7 +1308,16 @@
 CheckArrayType(Entity, IList, DeclType, Zero,
SubobjectIsDesignatorContext, Index,
StructuredList, StructuredIndex);
-  } else if (DeclType->isVoidType() || DeclType->isFunctionType()) {
+  } else if (DeclType->isVoidType()) {
+// [expr.type.conv]p2: Otherwise, if the type is cv void and the
+// initializer is () or {} (after pack expansion, if any), the expression
+// is a prvalue of type void that performs no initialization.
+if (IList->getNumInits() > 0) {
+  if (!VerifyOnly)
+SemaRef.Diag(IList->getBeginLoc(), diag::err_init_list_void_nonempty);
+  hadError = true;
+}
+  } else if (DeclType->isFunctionType()) {
 // This type is invalid, issue a diagnostic.
 ++Index;
 if (!VerifyOnly)
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5845,6 +5845,8 @@
 def err_illegal_initializer : Error<
   "illegal initializer (only variables can be initialized)">;
 def err_illegal_initializer_type : Error<"illegal initializer type %0">;
+def err_init_list_void_nonempty : Error<
+  "initializer list for 'void' must be empty">;
 def ext_init_list_type_narrowing : ExtWarn<
   "type %0 cannot be narrowed to %1 in initializer list">,
   InGroup, DefaultError, SFINAEFailure;


Index: clang/test/SemaCXX/cxx2a-explicit-bool.cpp
===
--- clang/test/SemaCXX/cxx2a-explicit-bool.cpp
+++ clang/test/SemaCXX/cxx2a-explicit-bool.cpp
@@ -73,8 +73,8 @@
 
 template  struct E {
   // expected-note@-1+ {{candidate constructor}}
-  explicit((T{}, false))
-  // expected-error@-1 {{illegal initializer type 'void'}}
+  explicit(((T&&)T{}, false))
+  // expected-error@-1 {{cannot form a reference to 'void'}}
   E(int);
 };
 
Index: clang/test/SemaCXX/attr-annotate.cpp
===
--- clang/test/SemaCXX/attr-annotate.cpp
+++ clang/test/SemaCXX/attr-annotate.cpp
@@ -42,8 +42,8 @@
 
   template
   struct B {
-[[clang::annotate("test", ((void)T{}, 9))]] void t() {}
-// expected-error@-1 {{illegal initializer type 'void'}}
+[[clang::annotate("test", ((void)T{1}, 9))]] void t() {}
+// expected-error@-1 {{initializer list for 'void' must be empty}}
   };
   B b;
   B b1;
Index: clang/test/CXX/expr/expr.post/expr.type.conv/p2.cpp
===
--- 

[PATCH] D113837: Sema: Let InitListExpr have dependent type instead of 'void'

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert updated this revision to Diff 387054.
aaronpuchert added a comment.
Herald added subscribers: kbarton, nemanjai.
Herald added a project: clang-tools-extra.

Adapt `clang-tidy/checkers/cppcoreguidelines-owning-memory.cpp` and run 
clang-format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113837

Files:
  clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-owning-memory.cpp
  clang/include/clang/AST/Expr.h
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/test/AST/ast-dump-decl.cpp
  clang/test/AST/ast-dump-recovery.cpp
  clang/test/AST/ast-dump-stmt-json.cpp
  clang/test/SemaOpenCLCXX/address-space-references.clcpp

Index: clang/test/SemaOpenCLCXX/address-space-references.clcpp
===
--- clang/test/SemaOpenCLCXX/address-space-references.clcpp
+++ clang/test/SemaOpenCLCXX/address-space-references.clcpp
@@ -22,7 +22,7 @@
   bar(1); // expected-error{{binding reference of type 'const __global unsigned int' to value of type 'int' changes address space}}
   C c;
   c.gen({1, 2});
-  c.glob({1, 2}); //expected-error{{binding reference of type 'const __global short2' (vector of 2 'short' values) to value of type 'void' changes address space}}
+  c.glob({1, 2}); //expected-error{{binding reference of type 'const __global short2' (vector of 2 'short' values) to value of type '' changes address space}}
   c.nested_list({{1, 2}, {3, 4}});
 }
 
Index: clang/test/AST/ast-dump-stmt-json.cpp
===
--- clang/test/AST/ast-dump-stmt-json.cpp
+++ clang/test/AST/ast-dump-stmt-json.cpp
@@ -2455,7 +2455,7 @@
 // CHECK-NEXT: }
 // CHECK-NEXT:},
 // CHECK-NEXT:"type": {
-// CHECK-NEXT: "qualType": "void"
+// CHECK-NEXT: "qualType": ""
 // CHECK-NEXT:},
 // CHECK-NEXT:"valueCategory": "prvalue"
 // CHECK-NEXT:   }
Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -197,7 +197,7 @@
   Bar b3 = Bar(x);
   // CHECK: `-VarDecl {{.*}} b4 'Bar'
   // CHECK-NEXT:  `-RecoveryExpr {{.*}} 'Bar' contains-errors
-  // CHECK-NEXT:`-InitListExpr {{.*}} 'void'
+  // CHECK-NEXT:`-InitListExpr {{.*}} ''
   // CHECK-NEXT:  `-DeclRefExpr {{.*}} 'x' 'int'
   Bar b4 = Bar{x};
   // CHECK: `-VarDecl {{.*}} b5 'Bar'
Index: clang/test/AST/ast-dump-decl.cpp
===
--- clang/test/AST/ast-dump-decl.cpp
+++ clang/test/AST/ast-dump-decl.cpp
@@ -607,10 +607,10 @@
   // CHECK:  VarTemplateDecl 0x{{.+}} parent 0x{{.+}} prev 0x{{.+}} <{{.+}}:[[@LINE-23]]:3, line:[[@LINE-22]]:34> col:14 TestVarTemplate
   // CHECK-NEXT: |-TemplateTypeParmDecl 0x{{.+}}  col:21 referenced typename depth 0 index 0 T
   // CHECK-NEXT: |-VarDecl 0x{{.+}} parent 0x{{.+}} prev 0x{{.+}}  col:14 TestVarTemplate 'const T' cinit
-  // CHECK-NEXT: | `-InitListExpr 0x{{.+}}  'void'
+  // CHECK-NEXT: | `-InitListExpr 0x{{.+}}  ''
   // CHECK-NEXT: |-VarTemplateSpecialization 0x{{.+}} 'TestVarTemplate' 'const int':'const int'
   // CHECK-NEXT: `-VarTemplateSpecialization 0x{{.+}} 'TestVarTemplate' 'const int':'const int'
-
+
   // CHECK:  VarTemplateSpecializationDecl 0x{{.+}} parent 0x{{.+}} prev 0x{{.+}} <{{.+}}:[[@LINE-29]]:3, col:34> col:14 referenced TestVarTemplate 'const int':'const int' cinit
   // CHECK-NEXT: |-TemplateArgument type 'int'
   // CHECK-NEXT: | `-BuiltinType 0x{{.+}} 'int'
Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -780,6 +780,9 @@
   continue;
 }
 
+if (isa_and_nonnull(RetE))
+  continue; // Should have been diagnosed already, type is bogus.
+
 // FIXME: This is a poor diagnostic for ReturnStmts without expressions.
 // TODO: It's possible that the *first* return is the divergent one.
 Diag(RS->getBeginLoc(),
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -744,7 +744,7 @@
 InitListExpr *OuterILE,
 unsigned OuterIndex,
 bool FillWithNoInit) {
-  assert((ILE->getType() != SemaRef.Context.VoidTy) &&
+  assert((ILE->getType() != SemaRef.Context.DependentTy) &&
  "Should not have void type");
 
   // We don't need to do any checks when just filling NoInitExprs; that can't
Index: clang/lib/Sema/SemaExpr.cpp

[PATCH] D113830: [clang-tidy] Fix false positive in `readability-identifier-naming` check involving `override` attribute

[Salman Javed via Phabricator via cfe-commits]

salman-javed-nz added a comment.

Happy to take a look at this, and do some of the initial review legwork, but 
let's leave final approval to @aaron.ballman.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113830

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

[PATCH] D113837: Sema: Let InitListExpr have dependent type instead of 'void'

[Aaron Puchert via Phabricator via cfe-commits]

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

It was always just a placeholder, but not a particularly good one: the
expression certainly has a value in most cases, and 'void' can also not
serve as an indicator for an undeduced type because of void{}.

Instead I think the dependent type is a good fit: an initializer list
is an expression that can instantiate to expressions of arbitrary types:
One could say that {...} is a map

  T : Type → T, T ↦ T{...}

The type parameter can be explicitly specified or be deduced from an
implicit conversion target type. So one could for example view {1} as

  struct InitList1 {
template
operator T() { return T{1}; }
  };

That in itself isn't type-dependent, but note that we let InitListExprs
have the actual type they initialize in the end, so in this case 'T'
instead of 'InitList1'.

A test failure in CXX/expr/expr.prim/expr.prim.lambda/p4.cpp revealed
that we were emitting errors "return type ... must match previous
return type ... when lambda expression has unspecified explicit return
type" even after diagnosing InitListExprs with "cannot deduce lambda
return type from initializer list", which in this case didn't show up
because it accidentally matched with the actual 'void' of the other
return statements.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113837

Files:
  clang/include/clang/AST/Expr.h
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/test/AST/ast-dump-decl.cpp
  clang/test/AST/ast-dump-recovery.cpp
  clang/test/AST/ast-dump-stmt-json.cpp
  clang/test/SemaOpenCLCXX/address-space-references.clcpp

Index: clang/test/SemaOpenCLCXX/address-space-references.clcpp
===
--- clang/test/SemaOpenCLCXX/address-space-references.clcpp
+++ clang/test/SemaOpenCLCXX/address-space-references.clcpp
@@ -22,7 +22,7 @@
   bar(1); // expected-error{{binding reference of type 'const __global unsigned int' to value of type 'int' changes address space}}
   C c;
   c.gen({1, 2});
-  c.glob({1, 2}); //expected-error{{binding reference of type 'const __global short2' (vector of 2 'short' values) to value of type 'void' changes address space}}
+  c.glob({1, 2}); //expected-error{{binding reference of type 'const __global short2' (vector of 2 'short' values) to value of type '' changes address space}}
   c.nested_list({{1, 2}, {3, 4}});
 }
 
Index: clang/test/AST/ast-dump-stmt-json.cpp
===
--- clang/test/AST/ast-dump-stmt-json.cpp
+++ clang/test/AST/ast-dump-stmt-json.cpp
@@ -2455,7 +2455,7 @@
 // CHECK-NEXT: }
 // CHECK-NEXT:},
 // CHECK-NEXT:"type": {
-// CHECK-NEXT: "qualType": "void"
+// CHECK-NEXT: "qualType": ""
 // CHECK-NEXT:},
 // CHECK-NEXT:"valueCategory": "prvalue"
 // CHECK-NEXT:   }
Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -197,7 +197,7 @@
   Bar b3 = Bar(x);
   // CHECK: `-VarDecl {{.*}} b4 'Bar'
   // CHECK-NEXT:  `-RecoveryExpr {{.*}} 'Bar' contains-errors
-  // CHECK-NEXT:`-InitListExpr {{.*}} 'void'
+  // CHECK-NEXT:`-InitListExpr {{.*}} ''
   // CHECK-NEXT:  `-DeclRefExpr {{.*}} 'x' 'int'
   Bar b4 = Bar{x};
   // CHECK: `-VarDecl {{.*}} b5 'Bar'
Index: clang/test/AST/ast-dump-decl.cpp
===
--- clang/test/AST/ast-dump-decl.cpp
+++ clang/test/AST/ast-dump-decl.cpp
@@ -607,7 +607,7 @@
   // CHECK:  VarTemplateDecl 0x{{.+}} parent 0x{{.+}} prev 0x{{.+}} <{{.+}}:[[@LINE-23]]:3, line:[[@LINE-22]]:34> col:14 TestVarTemplate
   // CHECK-NEXT: |-TemplateTypeParmDecl 0x{{.+}}  col:21 referenced typename depth 0 index 0 T
   // CHECK-NEXT: |-VarDecl 0x{{.+}} parent 0x{{.+}} prev 0x{{.+}}  col:14 TestVarTemplate 'const T' cinit
-  // CHECK-NEXT: | `-InitListExpr 0x{{.+}}  'void'
+  // CHECK-NEXT: | `-InitListExpr 0x{{.+}}  ''
   // CHECK-NEXT: |-VarTemplateSpecialization 0x{{.+}} 'TestVarTemplate' 'const int':'const int'
   // CHECK-NEXT: `-VarTemplateSpecialization 0x{{.+}} 'TestVarTemplate' 'const int':'const int'
 
Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -780,6 +780,9 @@
   continue;
 }
 
+if (isa_and_nonnull(RetE))
+  continue; // Should have been diagnosed already, type is bogus.
+
 // FIXME: This is a poor diagnostic for ReturnStmts without expressions.
 // TODO: It's possible that the *first* return is the divergent one.
 

[PATCH] D113826: [clang-format][c++2b] support removal of the space between auto and {} in P0849R8

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

HazardyKnusperkeks added a comment.

In D113826#3129383 , @lichray wrote:

> Thanks for the patch! It works for me, both for the `auto{x}` and `new 
> auto{x}` cases. Some corner cases left:
>
>   void f() { T()->n = 1; }
>   void f() { T{}->n = 1; } // xxx
>   void g() { auto()->n = 0; }
>   void h() { auto{}->n = 0; }  // xxx
>
> The `/// xxx` cases are formatted as
>
>   void f() {
> T {  } -> n = 1;
>   }
>
> and
>
>   void h() {
> auto{  } -> n = 0;
>   }
>
> But this may not worth a fix (for now).

I'm sure someone will look into it, but this should not be handled by this 
change.


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

https://reviews.llvm.org/D113826

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

[PATCH] D113826: [clang-format][c++2b] support removal of the space between auto and {} in P0849R8

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

Quuxplusone added a comment.

In D113826#3129383 , @lichray wrote:

>   void f() {
> T {  } -> n = 1;
>   }
>
> and
>
>   void h() {
> auto{  } -> n = 0;
>   }
>
> But this may not worth a fix (for now).

Kibitzing: I don't think those cases matter to real code. But that spacing is 
//so bizarre// that it might be worth looking into anyway. Like, what does 
clang-format think `auto{}` even //is?// I can't think of any construct at 
all in C++ where we'd want to format it as `T { anything` or `auto{ anything`. 
I wonder if this means clang-format would misformat `T{1, 2}` as `T { 1, 2 }`! 
Basically the only reason to leave a space in the middle of `identifier {` is 
if you're concerned about cutesy macros like `IF_CONSTEVAL {`, but in those 
cases you should be looking for there to be a newline after the `{` anyway.


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

https://reviews.llvm.org/D113826

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

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

[Cameron Mulhern via Phabricator via cfe-commits]

csmulhern marked an inline comment as done.
csmulhern added a comment.

I don't have commit access, so please submit this change on my behalf.

Thanks for all your help on landing this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109557

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

[PATCH] D113832: reland: [VFS] Use original path when falling back to external FS

[Keith Smiley via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG86e2af8043c7: reland: [VFS] Use original path when falling 
back to external FS (authored by keith).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113832

Files:
  clang/test/VFS/relative-path-errors.c
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -1627,6 +1627,114 @@
   EXPECT_EQ(0, NumDiagnostics);
 }
 
+TEST_F(VFSFromYAMLTest, ReturnsRequestedPathVFSMiss) {
+  IntrusiveRefCntPtr BaseFS(
+  new vfs::InMemoryFileSystem);
+  BaseFS->addFile("//root/foo/a", 0,
+  MemoryBuffer::getMemBuffer("contents of a"));
+  ASSERT_FALSE(BaseFS->setCurrentWorkingDirectory("//root/foo"));
+  auto RemappedFS = vfs::RedirectingFileSystem::create(
+  {}, /*UseExternalNames=*/false, *BaseFS);
+
+  auto OpenedF = RemappedFS->openFileForRead("a");
+  ASSERT_FALSE(OpenedF.getError());
+  llvm::ErrorOr Name = (*OpenedF)->getName();
+  ASSERT_FALSE(Name.getError());
+  EXPECT_EQ("a", Name.get());
+
+  auto OpenedS = (*OpenedF)->status();
+  ASSERT_FALSE(OpenedS.getError());
+  EXPECT_EQ("a", OpenedS->getName());
+  EXPECT_FALSE(OpenedS->IsVFSMapped);
+
+  auto DirectS = RemappedFS->status("a");
+  ASSERT_FALSE(DirectS.getError());
+  EXPECT_EQ("a", DirectS->getName());
+  EXPECT_FALSE(DirectS->IsVFSMapped);
+
+  EXPECT_EQ(0, NumDiagnostics);
+}
+
+TEST_F(VFSFromYAMLTest, ReturnsExternalPathVFSHit) {
+  IntrusiveRefCntPtr BaseFS(
+  new vfs::InMemoryFileSystem);
+  BaseFS->addFile("//root/foo/realname", 0,
+  MemoryBuffer::getMemBuffer("contents of a"));
+  auto FS =
+  getFromYAMLString("{ 'use-external-names': true,\n"
+"  'roots': [\n"
+"{\n"
+"  'type': 'directory',\n"
+"  'name': '//root/foo',\n"
+"  'contents': [ {\n"
+"  'type': 'file',\n"
+"  'name': 'vfsname',\n"
+"  'external-contents': 'realname'\n"
+"}\n"
+"  ]\n"
+"}]}",
+BaseFS);
+  ASSERT_FALSE(FS->setCurrentWorkingDirectory("//root/foo"));
+
+  auto OpenedF = FS->openFileForRead("vfsname");
+  ASSERT_FALSE(OpenedF.getError());
+  llvm::ErrorOr Name = (*OpenedF)->getName();
+  ASSERT_FALSE(Name.getError());
+  EXPECT_EQ("realname", Name.get());
+
+  auto OpenedS = (*OpenedF)->status();
+  ASSERT_FALSE(OpenedS.getError());
+  EXPECT_EQ("realname", OpenedS->getName());
+  EXPECT_TRUE(OpenedS->IsVFSMapped);
+
+  auto DirectS = FS->status("vfsname");
+  ASSERT_FALSE(DirectS.getError());
+  EXPECT_EQ("realname", DirectS->getName());
+  EXPECT_TRUE(DirectS->IsVFSMapped);
+
+  EXPECT_EQ(0, NumDiagnostics);
+}
+
+TEST_F(VFSFromYAMLTest, ReturnsInternalPathVFSHit) {
+  IntrusiveRefCntPtr BaseFS(
+  new vfs::InMemoryFileSystem);
+  BaseFS->addFile("//root/foo/realname", 0,
+  MemoryBuffer::getMemBuffer("contents of a"));
+  auto FS =
+  getFromYAMLString("{ 'use-external-names': false,\n"
+"  'roots': [\n"
+"{\n"
+"  'type': 'directory',\n"
+"  'name': '//root/foo',\n"
+"  'contents': [ {\n"
+"  'type': 'file',\n"
+"  'name': 'vfsname',\n"
+"  'external-contents': 'realname'\n"
+"}\n"
+"  ]\n"
+"}]}",
+BaseFS);
+  ASSERT_FALSE(FS->setCurrentWorkingDirectory("//root/foo"));
+
+  auto OpenedF = FS->openFileForRead("vfsname");
+  ASSERT_FALSE(OpenedF.getError());
+  llvm::ErrorOr Name = (*OpenedF)->getName();
+  ASSERT_FALSE(Name.getError());
+  EXPECT_EQ("vfsname", Name.get());
+
+  auto OpenedS = (*OpenedF)->status();
+  ASSERT_FALSE(OpenedS.getError());
+  EXPECT_EQ("vfsname", OpenedS->getName());
+  EXPECT_TRUE(OpenedS->IsVFSMapped);
+
+  auto DirectS = FS->status("vfsname");
+  ASSERT_FALSE(DirectS.getError());
+  EXPECT_EQ("vfsname", DirectS->getName());
+  EXPECT_TRUE(DirectS->IsVFSMapped);
+
+  EXPECT_EQ(0, NumDiagnostics);
+}
+
 TEST_F(VFSFromYAMLTest, CaseInsensitive) {
   IntrusiveRefCntPtr Lower(new 

[clang] 86e2af8 - reland: [VFS] Use original path when falling back to external FS

[Keith Smiley via cfe-commits]

Author: Keith Smiley
Date: 2021-11-13T12:14:34-08:00
New Revision: 86e2af8043c7728710a711b623f27425801a36c3

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

LOG: reland: [VFS] Use original path when falling back to external FS

This reverts commit f0cf544d6f6fe6cbca4c07772998272d6bb433d8.

Just a small change to fix:

```
/home/buildbot/as-builder-4/llvm-clang-x86_64-expensive-checks-ubuntu/llvm-project/llvm/lib/Support/VirtualFileSystem.cpp:
 In static member function ‘static 
llvm::ErrorOr > 
llvm::vfs::File::getWithPath(llvm::ErrorOr >, 
const llvm::Twine&)’:
/home/buildbot/as-builder-4/llvm-clang-x86_64-expensive-checks-ubuntu/llvm-project/llvm/lib/Support/VirtualFileSystem.cpp:2084:10:
 error: could not convert ‘F’ from ‘std::unique_ptr’ to 
‘llvm::ErrorOr >’
   return F;
  ^
```

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

Added: 
clang/test/VFS/relative-path-errors.c

Modified: 
llvm/include/llvm/Support/VirtualFileSystem.h
llvm/lib/Support/VirtualFileSystem.cpp
llvm/unittests/Support/VirtualFileSystemTest.cpp

Removed: 




diff  --git a/clang/test/VFS/relative-path-errors.c 
b/clang/test/VFS/relative-path-errors.c
new file mode 100644
index 0..8d3773a5eb592
--- /dev/null
+++ b/clang/test/VFS/relative-path-errors.c
@@ -0,0 +1,11 @@
+// RUN: mkdir -p %t
+// RUN: cd %t
+// RUN: cp %s main.c
+// RUN: not %clang_cc1 main.c 2>&1 | FileCheck %s
+// RUN: echo '{"roots": [],"version": 0}' > %t.yaml
+// RUN: not %clang_cc1 -ivfsoverlay %t.yaml main.c 2>&1 | FileCheck %s
+// RUN: echo '{"version": 
0,"roots":[{"type":"directory","name":"%/t","contents":[{"type":"file","name":"vfsname",
 "external-contents":"main.c"}]}]}' > %t.yaml
+// RUN: not %clang_cc1 -ivfsoverlay %t.yaml vfsname 2>&1 | FileCheck %s
+
+// CHECK: {{^}}main.c:[[# @LINE + 1]]:1: error:
+foobarbaz

diff  --git a/llvm/include/llvm/Support/VirtualFileSystem.h 
b/llvm/include/llvm/Support/VirtualFileSystem.h
index 38f426ff04432..10d2389ee079d 100644
--- a/llvm/include/llvm/Support/VirtualFileSystem.h
+++ b/llvm/include/llvm/Support/VirtualFileSystem.h
@@ -121,6 +121,14 @@ class File {
 
   /// Closes the file.
   virtual std::error_code close() = 0;
+
+  // Get the same file with a 
diff erent path.
+  static ErrorOr>
+  getWithPath(ErrorOr> Result, const Twine );
+
+protected:
+  // Set the file's underlying path.
+  virtual void setPath(const Twine ) {}
 };
 
 /// A member of a directory, yielded by a directory_iterator.
@@ -757,6 +765,12 @@ class RedirectingFileSystem : public vfs::FileSystem {
   /// with the given error code on a path associated with the provided Entry.
   bool shouldFallBackToExternalFS(std::error_code EC, Entry *E = nullptr) 
const;
 
+  /// Get the File status, or error, from the underlying external file system.
+  /// This returns the status with the originally requested name, while looking
+  /// up the entry using the canonical path.
+  ErrorOr getExternalStatus(const Twine ,
+const Twine ) const;
+
   // In a RedirectingFileSystem, keys can be specified in Posix or Windows
   // style (or even a mixture of both), so this comparison helper allows
   // slashes (representing a root) to match backslashes (and vice versa).  Note
@@ -814,7 +828,8 @@ class RedirectingFileSystem : public vfs::FileSystem {
Entry *From) const;
 
   /// Get the status for a path with the provided \c LookupResult.
-  ErrorOr status(const Twine , const LookupResult );
+  ErrorOr status(const Twine , const Twine ,
+ const LookupResult );
 
 public:
   /// Looks up \p Path in \c Roots and returns a LookupResult giving the

diff  --git a/llvm/lib/Support/VirtualFileSystem.cpp 
b/llvm/lib/Support/VirtualFileSystem.cpp
index a4abfe19bcbdc..9bf0384b5f1b0 100644
--- a/llvm/lib/Support/VirtualFileSystem.cpp
+++ b/llvm/lib/Support/VirtualFileSystem.cpp
@@ -194,6 +194,7 @@ class RealFile : public File {
bool RequiresNullTerminator,
bool IsVolatile) override;
   std::error_code close() override;
+  void setPath(const Twine ) override;
 };
 
 } // namespace
@@ -229,6 +230,12 @@ std::error_code RealFile::close() {
   return EC;
 }
 
+void RealFile::setPath(const Twine ) {
+  RealName = Path.str();
+  if (auto Status = status())
+S = Status.get().copyWithNewName(Status.get(), Path);
+}
+
 namespace {
 
 /// A file system according to your operating system.
@@ -639,6 +646,8 @@ class InMemoryFileAdaptor : public File {
   }
 
   std::error_code close() override { return {}; }
+
+  void setPath(const Twine ) override { RequestedName = Path.str(); }
 };
 } // namespace
 
@@ -1244,7 

[clang-tools-extra] 8ac9d2a - [clangd] Fix function-arg-placeholder suppression with macros.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2021-11-13T20:50:51+01:00
New Revision: 8ac9d2ae5839172013ac6e9108398902da8a8969

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

LOG: [clangd] Fix function-arg-placeholder suppression with macros.

While here, unhide function-arg-placeholders flag. It's reasonable to want and
maybe we should consider making it default.

Fixes https://github.com/clangd/clangd/issues/922

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

Added: 


Modified: 
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/tool/ClangdMain.cpp
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/CodeComplete.cpp 
b/clang-tools-extra/clangd/CodeComplete.cpp
index f1f1cae690bc8..f033b5ec001d8 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -466,32 +466,27 @@ struct CodeCompletionBuilder {
   // FIXME(ibiryukov): sometimes add template arguments to a snippet, e.g.
   // we need to complete 'forward<$1>($0)'.
   return "($0)";
-// Suppress function argument snippets cursor is followed by left
-// parenthesis (and potentially arguments) or if there are potentially
-// template arguments. There are cases where it would be wrong (e.g. next
-// '<' token is a comparison rather than template argument list start) but
-// it is less common and suppressing snippet provides better UX.
-if (Completion.Kind == CompletionItemKind::Function ||
-Completion.Kind == CompletionItemKind::Method ||
-Completion.Kind == CompletionItemKind::Constructor) {
-  // If there is a potential template argument list, drop snippet and just
-  // complete symbol name. Ideally, this could generate an edit that would
-  // paste function arguments after template argument list but it would be
-  // complicated. Example:
-  //
-  // fu^ -> function
+
+bool MayHaveArgList = Completion.Kind == CompletionItemKind::Function ||
+  Completion.Kind == CompletionItemKind::Method ||
+  Completion.Kind == CompletionItemKind::Constructor ||
+  Completion.Kind == CompletionItemKind::Text 
/*Macro*/;
+// If likely arg list already exists, don't add new parens & placeholders.
+//   Snippet: function(int x, int y)
+//   func^(1,2) -> function(1, 2)
+// NOT function(int x, int y)(1, 2)
+if (MayHaveArgList) {
+  // Check for a template argument list in the code.
+  //   Snippet: function(int x)
+  //   fu^(1) -> function(1)
   if (NextTokenKind == tok::less && Snippet->front() == '<')
 return "";
-  // Potentially followed by argument list.
+  // Potentially followed by regular argument list.
   if (NextTokenKind == tok::l_paren) {
-// If snippet contains template arguments we will emit them and drop
-// function arguments. Example:
-//
-// fu^(42) -> function(42);
+//   Snippet: function(int x)
+//   fu^(1,2) -> function(1, 2)
 if (Snippet->front() == '<') {
-  // Find matching '>'. Snippet->find('>') will not work in cases like
-  // template >. Hence, iterate through
-  // the snippet until the angle bracket balance reaches zero.
+  // Find matching '>', handling nested brackets.
   int Balance = 0;
   size_t I = 0;
   do {
@@ -512,8 +507,7 @@ struct CodeCompletionBuilder {
 // Replace argument snippets with a simplified pattern.
 if (Snippet->empty())
   return "";
-if (Completion.Kind == CompletionItemKind::Function ||
-Completion.Kind == CompletionItemKind::Method) {
+if (MayHaveArgList) {
   // Functions snippets can be of 2 types:
   // - containing only function arguments, e.g.
   //   foo(${1:int p1}, ${2:int p2});

diff  --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp 
b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index c6cf5c3d6e9a3..08631a31eda6c 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -233,7 +233,6 @@ opt EnableFunctionArgSnippets{
  "function calls. When enabled, completions also contain "
  "placeholders for method parameters"),
 init(CodeCompleteOptions().EnableFunctionArgSnippets),
-Hidden,
 };
 
 opt HeaderInsertion{

diff  --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp 
b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index aff6f6cf25ff9..81dfdcb371fb4 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ 

[PATCH] D113765: [clangd] Fix function-arg-placeholder suppression with macros.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8ac9d2ae5839: [clangd] Fix function-arg-placeholder 
suppression with macros. (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113765

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

Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1662,6 +1662,9 @@
   // Overload with bool
   int a(bool);
   int b(float);
+
+  X(int);
+  X(float);
 };
 int GFuncC(int);
 int GFuncD(int);
@@ -1671,6 +1674,10 @@
   EXPECT_THAT(completions(Context + "int y = X().^", {}, Opts).Completions,
   UnorderedElementsAre(Labeled("a(…)"), Labeled("b(float)")));
 
+  // Constructor completions are bundled.
+  EXPECT_THAT(completions(Context + "X z = X^", {}, Opts).Completions,
+  UnorderedElementsAre(Labeled("X"), Labeled("X(…)")));
+
   // Non-member completions are bundled, including index+sema.
   Symbol NoArgsGFunc = func("GFuncC");
   EXPECT_THAT(
@@ -2323,6 +2330,15 @@
 UnorderedElementsAre(AllOf(Named("foo_class"), SnippetSuffix("<$0>")),
  AllOf(Named("foo_alias"), SnippetSuffix("<$0>";
   }
+  {
+auto Results = completions(
+R"cpp(
+  #define FOO(x, y) x##f
+  FO^ )cpp",
+{}, Opts);
+EXPECT_THAT(Results.Completions, UnorderedElementsAre(AllOf(
+ Named("FOO"), SnippetSuffix("($0)";
+  }
 }
 
 TEST(CompletionTest, SuggestOverrides) {
@@ -3170,6 +3186,7 @@
   clangd::CodeCompleteOptions Opts;
   Opts.EnableSnippets = true;
   std::string Context = R"cpp(
+#define MACRO(x)
 int foo(int A);
 int bar();
 struct Object {
@@ -3217,6 +3234,9 @@
   Contains(AllOf(Labeled("Container(int Size)"),
  SnippetSuffix(""),
  Kind(CompletionItemKind::Constructor;
+  EXPECT_THAT(completions(Context + "MAC^(2)", {}, Opts).Completions,
+  Contains(AllOf(Labeled("MACRO(x)"), SnippetSuffix(""),
+ Kind(CompletionItemKind::Text;
 }
 
 TEST(CompletionTest, NoCrashDueToMacroOrdering) {
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -233,7 +233,6 @@
  "function calls. When enabled, completions also contain "
  "placeholders for method parameters"),
 init(CodeCompleteOptions().EnableFunctionArgSnippets),
-Hidden,
 };
 
 opt HeaderInsertion{
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -466,32 +466,27 @@
   // FIXME(ibiryukov): sometimes add template arguments to a snippet, e.g.
   // we need to complete 'forward<$1>($0)'.
   return "($0)";
-// Suppress function argument snippets cursor is followed by left
-// parenthesis (and potentially arguments) or if there are potentially
-// template arguments. There are cases where it would be wrong (e.g. next
-// '<' token is a comparison rather than template argument list start) but
-// it is less common and suppressing snippet provides better UX.
-if (Completion.Kind == CompletionItemKind::Function ||
-Completion.Kind == CompletionItemKind::Method ||
-Completion.Kind == CompletionItemKind::Constructor) {
-  // If there is a potential template argument list, drop snippet and just
-  // complete symbol name. Ideally, this could generate an edit that would
-  // paste function arguments after template argument list but it would be
-  // complicated. Example:
-  //
-  // fu^ -> function
+
+bool MayHaveArgList = Completion.Kind == CompletionItemKind::Function ||
+  Completion.Kind == CompletionItemKind::Method ||
+  Completion.Kind == CompletionItemKind::Constructor ||
+  Completion.Kind == CompletionItemKind::Text /*Macro*/;
+// If likely arg list already exists, don't add new parens & placeholders.
+//   Snippet: function(int x, int y)
+//   func^(1,2) -> function(1, 2)
+// NOT function(int x, int y)(1, 2)
+if (MayHaveArgList) {
+  // Check for a template argument list in the code.
+  //   Snippet: function(int x)
+  //   fu^(1) -> function(1)
   if (NextTokenKind == 

[PATCH] D113826: [clang-format][c++2b] support removal of the space between auto and {} in P0849R8

[Zhihao Yuan via Phabricator via cfe-commits]

lichray added a comment.

Thanks for the patch! It works for me, both for the `auto{x}` and `new auto{x}` 
cases. Some corner cases left:

  void f() { T()->n = 1; }
  void f() { T{}->n = 1; } // xxx
  void g() { auto()->n = 0; }
  void h() { auto{}->n = 0; }  // xxx

The `/// xxx` cases are formatted as

  void f() {
T {  } -> n = 1;
  }

and

  void h() {
auto{  } -> n = 0;
  }

But this may not worth a fix.


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

https://reviews.llvm.org/D113826

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

[PATCH] D95168: [clang-format] Add Insert/Remove Braces option

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

HazardyKnusperkeks added a comment.

Should I copy my comments on the old file?




Comment at: clang/lib/Format/BraceInserter.cpp:21-56
+bool SupportsAnyAutomaticBraces(FormatStyle , bool insertion) {
+  if (insertion) {
+if (Style.AutomaticBraces.AfterIf == FormatStyle::BIS_Always)
+  return true;
+if (Style.AutomaticBraces.AfterIf == FormatStyle::BIS_WrapLikely)
+  return true;
+if (Style.AutomaticBraces.AfterFor == FormatStyle::BIS_Always)

MyDeveloperDay wrote:
> HazardyKnusperkeks wrote:
> > How about something like that?
> I think for now
> 
> ```
> bool SupportsAnyAutomaticBraces(FormatStyle , bool insertion) {
>   if (   Style.AutomaticBraces.AfterIf == FormatStyle::BIS_Leave
>   && Style.AutomaticBraces.AfterFor == FormatStyle::BIS_Leave
>   && Style.AutomaticBraces.AfterWhile == FormatStyle::BIS_Leave
>   && Style.AutomaticBraces.AfterDo == FormatStyle::BIS_Leave
>   && Style.AutomaticBraces.AfterElse == FormatStyle::BIS_Leave)
>   return false;
> 
>   if (!insertion){
> if (Style.AutomaticBraces.AfterIf != FormatStyle::BIS_Remove)
> && Style.AutomaticBraces.AfterFor != FormatStyle::BIS_Remove)
> && Style.AutomaticBraces.AfterWhile != FormatStyle::BIS_Remove)
> && Style.AutomaticBraces.AfterDo != FormatStyle::BIS_Remove)
> && Style.AutomaticBraces.AfterElse != FormatStyle::BIS_Remove)
>   return false;
>   }
>   return true;
> }
> ```
> 
> Should cover it.
Yeah, but I like my variant better. :)



Comment at: clang/lib/Format/Format.cpp:2461
 
-  if (Style.SortIncludes == FormatStyle::SI_CaseInsensitive) {
+  if (Style.SortIncludes == FormatStyle::SI_CaseInsensitive)
 llvm::stable_sort(Indices, [&](unsigned LHSI, unsigned RHSI) {

I think this should keep its brace.


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

https://reviews.llvm.org/D95168

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

[PATCH] D113826: [clang-format][c++2b] support removal of the space between auto and {} in P0849R8

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

HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added a comment.

Looks good. :)

In D113826#3129229 , @Quuxplusone 
wrote:

> Personally I'd remove the tests that are invalid expressions; but I don't 
> know what's the usual practice. Ship it AFAIC!

It's nice if even invalid code gets formatted, because some of us have 
formatting on typing, and the then invalid code may become valid very quickly 
and it would be nice if the code does not jump around. :)


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

https://reviews.llvm.org/D113826

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

[PATCH] D113832: reland: [VFS] Use original path when falling back to external FS

[Keith Smiley via Phabricator via cfe-commits]

keith created this revision.
keith added a reviewer: dexonsmith.
Herald added subscribers: pengfei, hiraditya.
keith requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This reverts commit f0cf544d6f6fe6cbca4c07772998272d6bb433d8 
.

Just a small change to fix:

  
/home/buildbot/as-builder-4/llvm-clang-x86_64-expensive-checks-ubuntu/llvm-project/llvm/lib/Support/VirtualFileSystem.cpp:
 In static member function ‘static 
llvm::ErrorOr > 
llvm::vfs::File::getWithPath(llvm::ErrorOr >, 
const llvm::Twine&)’:
  
/home/buildbot/as-builder-4/llvm-clang-x86_64-expensive-checks-ubuntu/llvm-project/llvm/lib/Support/VirtualFileSystem.cpp:2084:10:
 error: could not convert ‘F’ from ‘std::unique_ptr’ to 
‘llvm::ErrorOr >’
 return F;
^


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113832

Files:
  clang/test/VFS/relative-path-errors.c
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -1627,6 +1627,114 @@
   EXPECT_EQ(0, NumDiagnostics);
 }
 
+TEST_F(VFSFromYAMLTest, ReturnsRequestedPathVFSMiss) {
+  IntrusiveRefCntPtr BaseFS(
+  new vfs::InMemoryFileSystem);
+  BaseFS->addFile("//root/foo/a", 0,
+  MemoryBuffer::getMemBuffer("contents of a"));
+  ASSERT_FALSE(BaseFS->setCurrentWorkingDirectory("//root/foo"));
+  auto RemappedFS = vfs::RedirectingFileSystem::create(
+  {}, /*UseExternalNames=*/false, *BaseFS);
+
+  auto OpenedF = RemappedFS->openFileForRead("a");
+  ASSERT_FALSE(OpenedF.getError());
+  llvm::ErrorOr Name = (*OpenedF)->getName();
+  ASSERT_FALSE(Name.getError());
+  EXPECT_EQ("a", Name.get());
+
+  auto OpenedS = (*OpenedF)->status();
+  ASSERT_FALSE(OpenedS.getError());
+  EXPECT_EQ("a", OpenedS->getName());
+  EXPECT_FALSE(OpenedS->IsVFSMapped);
+
+  auto DirectS = RemappedFS->status("a");
+  ASSERT_FALSE(DirectS.getError());
+  EXPECT_EQ("a", DirectS->getName());
+  EXPECT_FALSE(DirectS->IsVFSMapped);
+
+  EXPECT_EQ(0, NumDiagnostics);
+}
+
+TEST_F(VFSFromYAMLTest, ReturnsExternalPathVFSHit) {
+  IntrusiveRefCntPtr BaseFS(
+  new vfs::InMemoryFileSystem);
+  BaseFS->addFile("//root/foo/realname", 0,
+  MemoryBuffer::getMemBuffer("contents of a"));
+  auto FS =
+  getFromYAMLString("{ 'use-external-names': true,\n"
+"  'roots': [\n"
+"{\n"
+"  'type': 'directory',\n"
+"  'name': '//root/foo',\n"
+"  'contents': [ {\n"
+"  'type': 'file',\n"
+"  'name': 'vfsname',\n"
+"  'external-contents': 'realname'\n"
+"}\n"
+"  ]\n"
+"}]}",
+BaseFS);
+  ASSERT_FALSE(FS->setCurrentWorkingDirectory("//root/foo"));
+
+  auto OpenedF = FS->openFileForRead("vfsname");
+  ASSERT_FALSE(OpenedF.getError());
+  llvm::ErrorOr Name = (*OpenedF)->getName();
+  ASSERT_FALSE(Name.getError());
+  EXPECT_EQ("realname", Name.get());
+
+  auto OpenedS = (*OpenedF)->status();
+  ASSERT_FALSE(OpenedS.getError());
+  EXPECT_EQ("realname", OpenedS->getName());
+  EXPECT_TRUE(OpenedS->IsVFSMapped);
+
+  auto DirectS = FS->status("vfsname");
+  ASSERT_FALSE(DirectS.getError());
+  EXPECT_EQ("realname", DirectS->getName());
+  EXPECT_TRUE(DirectS->IsVFSMapped);
+
+  EXPECT_EQ(0, NumDiagnostics);
+}
+
+TEST_F(VFSFromYAMLTest, ReturnsInternalPathVFSHit) {
+  IntrusiveRefCntPtr BaseFS(
+  new vfs::InMemoryFileSystem);
+  BaseFS->addFile("//root/foo/realname", 0,
+  MemoryBuffer::getMemBuffer("contents of a"));
+  auto FS =
+  getFromYAMLString("{ 'use-external-names': false,\n"
+"  'roots': [\n"
+"{\n"
+"  'type': 'directory',\n"
+"  'name': '//root/foo',\n"
+"  'contents': [ {\n"
+"  'type': 'file',\n"
+"  'name': 'vfsname',\n"
+"  'external-contents': 'realname'\n"
+"}\n"
+"  ]\n"
+"}]}",
+BaseFS);
+  ASSERT_FALSE(FS->setCurrentWorkingDirectory("//root/foo"));
+
+  auto OpenedF = FS->openFileForRead("vfsname");
+  ASSERT_FALSE(OpenedF.getError());
+  

[PATCH] D113765: [clangd] Fix function-arg-placeholder suppression with macros.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:517
+Completion.Kind == CompletionItemKind::Method ||
+Completion.Kind == CompletionItemKind::Text /*Macro*/) {
   // Functions snippets can be of 2 types:

sammccall wrote:
> kadircet wrote:
> > again while here, maybe introduce constructors into this condition. (Even 
> > better, what about a `bool isFunctionLikeCompletion(const CompletionItem&)` 
> > that we can use both here and above?)
> Done, though I didn't want to extract the variable too far away as there are 
> some wrinkles (macros may not be function-like, template args...) that we can 
> get away without resolving here.
thanks! yes that makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113765

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

[clang] f0cf544 - Revert "[VFS] Use original path when falling back to external FS"

[Keith Smiley via cfe-commits]

Author: Keith Smiley
Date: 2021-11-13T10:11:51-08:00
New Revision: f0cf544d6f6fe6cbca4c07772998272d6bb433d8

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

LOG: Revert "[VFS] Use original path when falling back to external FS"

```
/work/omp-vega20-0/openmp-offload-amdgpu-runtime/llvm.src/llvm/lib/Support/VirtualFileSystem.cpp:
 In static member function 'static 
llvm::ErrorOr > 
llvm::vfs::File::getWithPath(llvm::ErrorOr >, 
const llvm::Twine&)':
/work/omp-vega20-0/openmp-offload-amdgpu-runtime/llvm.src/llvm/lib/Support/VirtualFileSystem.cpp:2084:10:
 error: could not convert 'F' from 'std::unique_ptr' to 
'llvm::ErrorOr >'
   return F;
  ^
```

This reverts commit c972175649f4bb50d40d911659a04d5620ce6fe0.

Added: 


Modified: 
llvm/include/llvm/Support/VirtualFileSystem.h
llvm/lib/Support/VirtualFileSystem.cpp
llvm/unittests/Support/VirtualFileSystemTest.cpp

Removed: 
clang/test/VFS/relative-path-errors.c



diff  --git a/clang/test/VFS/relative-path-errors.c 
b/clang/test/VFS/relative-path-errors.c
deleted file mode 100644
index 8d3773a5eb59..
--- a/clang/test/VFS/relative-path-errors.c
+++ /dev/null
@@ -1,11 +0,0 @@
-// RUN: mkdir -p %t
-// RUN: cd %t
-// RUN: cp %s main.c
-// RUN: not %clang_cc1 main.c 2>&1 | FileCheck %s
-// RUN: echo '{"roots": [],"version": 0}' > %t.yaml
-// RUN: not %clang_cc1 -ivfsoverlay %t.yaml main.c 2>&1 | FileCheck %s
-// RUN: echo '{"version": 
0,"roots":[{"type":"directory","name":"%/t","contents":[{"type":"file","name":"vfsname",
 "external-contents":"main.c"}]}]}' > %t.yaml
-// RUN: not %clang_cc1 -ivfsoverlay %t.yaml vfsname 2>&1 | FileCheck %s
-
-// CHECK: {{^}}main.c:[[# @LINE + 1]]:1: error:
-foobarbaz

diff  --git a/llvm/include/llvm/Support/VirtualFileSystem.h 
b/llvm/include/llvm/Support/VirtualFileSystem.h
index 10d2389ee079..38f426ff0443 100644
--- a/llvm/include/llvm/Support/VirtualFileSystem.h
+++ b/llvm/include/llvm/Support/VirtualFileSystem.h
@@ -121,14 +121,6 @@ class File {
 
   /// Closes the file.
   virtual std::error_code close() = 0;
-
-  // Get the same file with a 
diff erent path.
-  static ErrorOr>
-  getWithPath(ErrorOr> Result, const Twine );
-
-protected:
-  // Set the file's underlying path.
-  virtual void setPath(const Twine ) {}
 };
 
 /// A member of a directory, yielded by a directory_iterator.
@@ -765,12 +757,6 @@ class RedirectingFileSystem : public vfs::FileSystem {
   /// with the given error code on a path associated with the provided Entry.
   bool shouldFallBackToExternalFS(std::error_code EC, Entry *E = nullptr) 
const;
 
-  /// Get the File status, or error, from the underlying external file system.
-  /// This returns the status with the originally requested name, while looking
-  /// up the entry using the canonical path.
-  ErrorOr getExternalStatus(const Twine ,
-const Twine ) const;
-
   // In a RedirectingFileSystem, keys can be specified in Posix or Windows
   // style (or even a mixture of both), so this comparison helper allows
   // slashes (representing a root) to match backslashes (and vice versa).  Note
@@ -828,8 +814,7 @@ class RedirectingFileSystem : public vfs::FileSystem {
Entry *From) const;
 
   /// Get the status for a path with the provided \c LookupResult.
-  ErrorOr status(const Twine , const Twine ,
- const LookupResult );
+  ErrorOr status(const Twine , const LookupResult );
 
 public:
   /// Looks up \p Path in \c Roots and returns a LookupResult giving the

diff  --git a/llvm/lib/Support/VirtualFileSystem.cpp 
b/llvm/lib/Support/VirtualFileSystem.cpp
index d0ba3114aa5c..a4abfe19bcbd 100644
--- a/llvm/lib/Support/VirtualFileSystem.cpp
+++ b/llvm/lib/Support/VirtualFileSystem.cpp
@@ -194,7 +194,6 @@ class RealFile : public File {
bool RequiresNullTerminator,
bool IsVolatile) override;
   std::error_code close() override;
-  void setPath(const Twine ) override;
 };
 
 } // namespace
@@ -230,12 +229,6 @@ std::error_code RealFile::close() {
   return EC;
 }
 
-void RealFile::setPath(const Twine ) {
-  RealName = Path.str();
-  if (auto Status = status())
-S = Status.get().copyWithNewName(Status.get(), Path);
-}
-
 namespace {
 
 /// A file system according to your operating system.
@@ -646,8 +639,6 @@ class InMemoryFileAdaptor : public File {
   }
 
   std::error_code close() override { return {}; }
-
-  void setPath(const Twine ) override { RequestedName = Path.str(); }
 };
 } // namespace
 
@@ -1253,7 +1244,7 @@ directory_iterator RedirectingFileSystem::dir_begin(const 
Twine ,
   }
 
   // Use status to make sure the path exists and refers 

[PATCH] D113830: [clang-tidy] Fix false positive in `readability-identifier-naming` check involving `override` attribute

[Fabian Wolff via Phabricator via cfe-commits]

fwolff created this revision.
fwolff added reviewers: alexfh, salman-javed-nz.
fwolff added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, xazax.hun.
fwolff requested review of this revision.
Herald added a subscriber: cfe-commits.

Fixes part of PR#45815. Overriding methods should not get a 
`readability-identifier-naming` warning because the issue can only be fixed in 
the base class; but the current check for whether a method is overriding does 
not take the `override` attribute into account, which makes a difference for 
dependent base classes.

The other issue mentioned in PR#45815 is not solved by this patch: Applying the 
fix provided by `readability-identifier-naming` only changes the name in the 
base class, not in the derived class(es) or at any call sites. This is 
difficult to fix, because in addition to the template base class, there could 
be specializations of the base class that also contain the overridden method, 
which is why applying the fix from the base class in the derived class in 
general would not lead to correct code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113830

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
@@ -305,9 +305,29 @@
   }
 };
 
-void VirtualCall(AOverridden _vItem) {
+template
+class ATOverridden {
+public:
+  virtual void BadBaseMethod() = 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual 
method 'BadBaseMethod'
+  // CHECK-FIXES: {{^}}  virtual void v_Bad_Base_Method() = 0;
+};
+
+template
+class CTOverriding : public ATOverridden {
+  // Overriding a badly-named base isn't a new violation.
+  // FIXME: The fixes from the base class should be propagated to the derived 
class here
+  //(note that there could be specializations of the template base 
class, though)
+  void BadBaseMethod() override{}
+};
+
+template
+void VirtualCall(AOverridden _vItem, ATOverridden _vTitem) {
   a_vItem.BadBaseMethod();
   // CHECK-FIXES: {{^}}  a_vItem.v_Bad_Base_Method();
+
+  // FIXME: The fixes from ATOverridden should be propagated to the following 
call
+  a_vTitem.BadBaseMethod();
 }
 
 template 
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -1257,7 +1257,7 @@
 
   if (const auto *Decl = dyn_cast(D)) {
 if (Decl->isMain() || !Decl->isUserProvided() ||
-Decl->size_overridden_methods() > 0)
+Decl->size_overridden_methods() > 0 || Decl->hasAttr())
   return SK_Invalid;
 
 // If this method has the same name as any base method, this is likely


Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
@@ -305,9 +305,29 @@
   }
 };
 
-void VirtualCall(AOverridden _vItem) {
+template
+class ATOverridden {
+public:
+  virtual void BadBaseMethod() = 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethod'
+  // CHECK-FIXES: {{^}}  virtual void v_Bad_Base_Method() = 0;
+};
+
+template
+class CTOverriding : public ATOverridden {
+  // Overriding a badly-named base isn't a new violation.
+  // FIXME: The fixes from the base class should be propagated to the derived class here
+  //(note that there could be specializations of the template base class, though)
+  void BadBaseMethod() override{}
+};
+
+template
+void VirtualCall(AOverridden _vItem, ATOverridden _vTitem) {
   a_vItem.BadBaseMethod();
   // CHECK-FIXES: {{^}}  a_vItem.v_Bad_Base_Method();
+
+  // FIXME: The fixes from ATOverridden should be propagated to the following call
+  a_vTitem.BadBaseMethod();
 }
 
 template 
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -1257,7 +1257,7 @@
 
   if (const auto *Decl = dyn_cast(D)) {
 if (Decl->isMain() || !Decl->isUserProvided() ||
-Decl->size_overridden_methods() > 0)
+Decl->size_overridden_methods() > 0 || Decl->hasAttr())
   return SK_Invalid;
 
  

[clang] c972175 - [VFS] Use original path when falling back to external FS

[Keith Smiley via cfe-commits]

Author: Keith Smiley
Date: 2021-11-13T09:34:44-08:00
New Revision: c972175649f4bb50d40d911659a04d5620ce6fe0

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

LOG: [VFS] Use original path when falling back to external FS

This is a follow up to 0be9ca7c0f9a733f846bb6bc4e8e36d46b518162 to make
paths in the case of falling back to the external file system use the
original format, preserving relative paths, and allow the external
filesystem to canonicalize them if needed.

Reviewed By: dexonsmith

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

Added: 
clang/test/VFS/relative-path-errors.c

Modified: 
llvm/include/llvm/Support/VirtualFileSystem.h
llvm/lib/Support/VirtualFileSystem.cpp
llvm/unittests/Support/VirtualFileSystemTest.cpp

Removed: 




diff  --git a/clang/test/VFS/relative-path-errors.c 
b/clang/test/VFS/relative-path-errors.c
new file mode 100644
index ..8d3773a5eb59
--- /dev/null
+++ b/clang/test/VFS/relative-path-errors.c
@@ -0,0 +1,11 @@
+// RUN: mkdir -p %t
+// RUN: cd %t
+// RUN: cp %s main.c
+// RUN: not %clang_cc1 main.c 2>&1 | FileCheck %s
+// RUN: echo '{"roots": [],"version": 0}' > %t.yaml
+// RUN: not %clang_cc1 -ivfsoverlay %t.yaml main.c 2>&1 | FileCheck %s
+// RUN: echo '{"version": 
0,"roots":[{"type":"directory","name":"%/t","contents":[{"type":"file","name":"vfsname",
 "external-contents":"main.c"}]}]}' > %t.yaml
+// RUN: not %clang_cc1 -ivfsoverlay %t.yaml vfsname 2>&1 | FileCheck %s
+
+// CHECK: {{^}}main.c:[[# @LINE + 1]]:1: error:
+foobarbaz

diff  --git a/llvm/include/llvm/Support/VirtualFileSystem.h 
b/llvm/include/llvm/Support/VirtualFileSystem.h
index 38f426ff0443..10d2389ee079 100644
--- a/llvm/include/llvm/Support/VirtualFileSystem.h
+++ b/llvm/include/llvm/Support/VirtualFileSystem.h
@@ -121,6 +121,14 @@ class File {
 
   /// Closes the file.
   virtual std::error_code close() = 0;
+
+  // Get the same file with a 
diff erent path.
+  static ErrorOr>
+  getWithPath(ErrorOr> Result, const Twine );
+
+protected:
+  // Set the file's underlying path.
+  virtual void setPath(const Twine ) {}
 };
 
 /// A member of a directory, yielded by a directory_iterator.
@@ -757,6 +765,12 @@ class RedirectingFileSystem : public vfs::FileSystem {
   /// with the given error code on a path associated with the provided Entry.
   bool shouldFallBackToExternalFS(std::error_code EC, Entry *E = nullptr) 
const;
 
+  /// Get the File status, or error, from the underlying external file system.
+  /// This returns the status with the originally requested name, while looking
+  /// up the entry using the canonical path.
+  ErrorOr getExternalStatus(const Twine ,
+const Twine ) const;
+
   // In a RedirectingFileSystem, keys can be specified in Posix or Windows
   // style (or even a mixture of both), so this comparison helper allows
   // slashes (representing a root) to match backslashes (and vice versa).  Note
@@ -814,7 +828,8 @@ class RedirectingFileSystem : public vfs::FileSystem {
Entry *From) const;
 
   /// Get the status for a path with the provided \c LookupResult.
-  ErrorOr status(const Twine , const LookupResult );
+  ErrorOr status(const Twine , const Twine ,
+ const LookupResult );
 
 public:
   /// Looks up \p Path in \c Roots and returns a LookupResult giving the

diff  --git a/llvm/lib/Support/VirtualFileSystem.cpp 
b/llvm/lib/Support/VirtualFileSystem.cpp
index a4abfe19bcbd..d0ba3114aa5c 100644
--- a/llvm/lib/Support/VirtualFileSystem.cpp
+++ b/llvm/lib/Support/VirtualFileSystem.cpp
@@ -194,6 +194,7 @@ class RealFile : public File {
bool RequiresNullTerminator,
bool IsVolatile) override;
   std::error_code close() override;
+  void setPath(const Twine ) override;
 };
 
 } // namespace
@@ -229,6 +230,12 @@ std::error_code RealFile::close() {
   return EC;
 }
 
+void RealFile::setPath(const Twine ) {
+  RealName = Path.str();
+  if (auto Status = status())
+S = Status.get().copyWithNewName(Status.get(), Path);
+}
+
 namespace {
 
 /// A file system according to your operating system.
@@ -639,6 +646,8 @@ class InMemoryFileAdaptor : public File {
   }
 
   std::error_code close() override { return {}; }
+
+  void setPath(const Twine ) override { RequestedName = Path.str(); }
 };
 } // namespace
 
@@ -1244,7 +1253,7 @@ directory_iterator RedirectingFileSystem::dir_begin(const 
Twine ,
   }
 
   // Use status to make sure the path exists and refers to a directory.
-  ErrorOr S = status(Path, *Result);
+  ErrorOr S = status(Path, Dir, *Result);
   if (!S) {
 if (shouldFallBackToExternalFS(S.getError(), Result->E))
  

[PATCH] D109128: [VFS] Use original path when falling back to external FS

[Keith Smiley 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 rGc972175649f4: [VFS] Use original path when falling back to 
external FS (authored by keith).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109128

Files:
  clang/test/VFS/relative-path-errors.c
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -1627,6 +1627,114 @@
   EXPECT_EQ(0, NumDiagnostics);
 }
 
+TEST_F(VFSFromYAMLTest, ReturnsRequestedPathVFSMiss) {
+  IntrusiveRefCntPtr BaseFS(
+  new vfs::InMemoryFileSystem);
+  BaseFS->addFile("//root/foo/a", 0,
+  MemoryBuffer::getMemBuffer("contents of a"));
+  ASSERT_FALSE(BaseFS->setCurrentWorkingDirectory("//root/foo"));
+  auto RemappedFS = vfs::RedirectingFileSystem::create(
+  {}, /*UseExternalNames=*/false, *BaseFS);
+
+  auto OpenedF = RemappedFS->openFileForRead("a");
+  ASSERT_FALSE(OpenedF.getError());
+  llvm::ErrorOr Name = (*OpenedF)->getName();
+  ASSERT_FALSE(Name.getError());
+  EXPECT_EQ("a", Name.get());
+
+  auto OpenedS = (*OpenedF)->status();
+  ASSERT_FALSE(OpenedS.getError());
+  EXPECT_EQ("a", OpenedS->getName());
+  EXPECT_FALSE(OpenedS->IsVFSMapped);
+
+  auto DirectS = RemappedFS->status("a");
+  ASSERT_FALSE(DirectS.getError());
+  EXPECT_EQ("a", DirectS->getName());
+  EXPECT_FALSE(DirectS->IsVFSMapped);
+
+  EXPECT_EQ(0, NumDiagnostics);
+}
+
+TEST_F(VFSFromYAMLTest, ReturnsExternalPathVFSHit) {
+  IntrusiveRefCntPtr BaseFS(
+  new vfs::InMemoryFileSystem);
+  BaseFS->addFile("//root/foo/realname", 0,
+  MemoryBuffer::getMemBuffer("contents of a"));
+  auto FS =
+  getFromYAMLString("{ 'use-external-names': true,\n"
+"  'roots': [\n"
+"{\n"
+"  'type': 'directory',\n"
+"  'name': '//root/foo',\n"
+"  'contents': [ {\n"
+"  'type': 'file',\n"
+"  'name': 'vfsname',\n"
+"  'external-contents': 'realname'\n"
+"}\n"
+"  ]\n"
+"}]}",
+BaseFS);
+  ASSERT_FALSE(FS->setCurrentWorkingDirectory("//root/foo"));
+
+  auto OpenedF = FS->openFileForRead("vfsname");
+  ASSERT_FALSE(OpenedF.getError());
+  llvm::ErrorOr Name = (*OpenedF)->getName();
+  ASSERT_FALSE(Name.getError());
+  EXPECT_EQ("realname", Name.get());
+
+  auto OpenedS = (*OpenedF)->status();
+  ASSERT_FALSE(OpenedS.getError());
+  EXPECT_EQ("realname", OpenedS->getName());
+  EXPECT_TRUE(OpenedS->IsVFSMapped);
+
+  auto DirectS = FS->status("vfsname");
+  ASSERT_FALSE(DirectS.getError());
+  EXPECT_EQ("realname", DirectS->getName());
+  EXPECT_TRUE(DirectS->IsVFSMapped);
+
+  EXPECT_EQ(0, NumDiagnostics);
+}
+
+TEST_F(VFSFromYAMLTest, ReturnsInternalPathVFSHit) {
+  IntrusiveRefCntPtr BaseFS(
+  new vfs::InMemoryFileSystem);
+  BaseFS->addFile("//root/foo/realname", 0,
+  MemoryBuffer::getMemBuffer("contents of a"));
+  auto FS =
+  getFromYAMLString("{ 'use-external-names': false,\n"
+"  'roots': [\n"
+"{\n"
+"  'type': 'directory',\n"
+"  'name': '//root/foo',\n"
+"  'contents': [ {\n"
+"  'type': 'file',\n"
+"  'name': 'vfsname',\n"
+"  'external-contents': 'realname'\n"
+"}\n"
+"  ]\n"
+"}]}",
+BaseFS);
+  ASSERT_FALSE(FS->setCurrentWorkingDirectory("//root/foo"));
+
+  auto OpenedF = FS->openFileForRead("vfsname");
+  ASSERT_FALSE(OpenedF.getError());
+  llvm::ErrorOr Name = (*OpenedF)->getName();
+  ASSERT_FALSE(Name.getError());
+  EXPECT_EQ("vfsname", Name.get());
+
+  auto OpenedS = (*OpenedF)->status();
+  ASSERT_FALSE(OpenedS.getError());
+  EXPECT_EQ("vfsname", OpenedS->getName());
+  EXPECT_TRUE(OpenedS->IsVFSMapped);
+
+  auto DirectS = FS->status("vfsname");
+  ASSERT_FALSE(DirectS.getError());
+  EXPECT_EQ("vfsname", DirectS->getName());
+  EXPECT_TRUE(DirectS->IsVFSMapped);
+
+  EXPECT_EQ(0, NumDiagnostics);
+}
+
 TEST_F(VFSFromYAMLTest, CaseInsensitive) {
   IntrusiveRefCntPtr Lower(new DummyFileSystem());
   

[PATCH] D109128: [VFS] Use original path when falling back to external FS

[Keith Smiley via Phabricator via cfe-commits]

keith updated this revision to Diff 387030.
keith added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109128

Files:
  clang/test/VFS/relative-path-errors.c
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -1627,6 +1627,114 @@
   EXPECT_EQ(0, NumDiagnostics);
 }
 
+TEST_F(VFSFromYAMLTest, ReturnsRequestedPathVFSMiss) {
+  IntrusiveRefCntPtr BaseFS(
+  new vfs::InMemoryFileSystem);
+  BaseFS->addFile("//root/foo/a", 0,
+  MemoryBuffer::getMemBuffer("contents of a"));
+  ASSERT_FALSE(BaseFS->setCurrentWorkingDirectory("//root/foo"));
+  auto RemappedFS = vfs::RedirectingFileSystem::create(
+  {}, /*UseExternalNames=*/false, *BaseFS);
+
+  auto OpenedF = RemappedFS->openFileForRead("a");
+  ASSERT_FALSE(OpenedF.getError());
+  llvm::ErrorOr Name = (*OpenedF)->getName();
+  ASSERT_FALSE(Name.getError());
+  EXPECT_EQ("a", Name.get());
+
+  auto OpenedS = (*OpenedF)->status();
+  ASSERT_FALSE(OpenedS.getError());
+  EXPECT_EQ("a", OpenedS->getName());
+  EXPECT_FALSE(OpenedS->IsVFSMapped);
+
+  auto DirectS = RemappedFS->status("a");
+  ASSERT_FALSE(DirectS.getError());
+  EXPECT_EQ("a", DirectS->getName());
+  EXPECT_FALSE(DirectS->IsVFSMapped);
+
+  EXPECT_EQ(0, NumDiagnostics);
+}
+
+TEST_F(VFSFromYAMLTest, ReturnsExternalPathVFSHit) {
+  IntrusiveRefCntPtr BaseFS(
+  new vfs::InMemoryFileSystem);
+  BaseFS->addFile("//root/foo/realname", 0,
+  MemoryBuffer::getMemBuffer("contents of a"));
+  auto FS =
+  getFromYAMLString("{ 'use-external-names': true,\n"
+"  'roots': [\n"
+"{\n"
+"  'type': 'directory',\n"
+"  'name': '//root/foo',\n"
+"  'contents': [ {\n"
+"  'type': 'file',\n"
+"  'name': 'vfsname',\n"
+"  'external-contents': 'realname'\n"
+"}\n"
+"  ]\n"
+"}]}",
+BaseFS);
+  ASSERT_FALSE(FS->setCurrentWorkingDirectory("//root/foo"));
+
+  auto OpenedF = FS->openFileForRead("vfsname");
+  ASSERT_FALSE(OpenedF.getError());
+  llvm::ErrorOr Name = (*OpenedF)->getName();
+  ASSERT_FALSE(Name.getError());
+  EXPECT_EQ("realname", Name.get());
+
+  auto OpenedS = (*OpenedF)->status();
+  ASSERT_FALSE(OpenedS.getError());
+  EXPECT_EQ("realname", OpenedS->getName());
+  EXPECT_TRUE(OpenedS->IsVFSMapped);
+
+  auto DirectS = FS->status("vfsname");
+  ASSERT_FALSE(DirectS.getError());
+  EXPECT_EQ("realname", DirectS->getName());
+  EXPECT_TRUE(DirectS->IsVFSMapped);
+
+  EXPECT_EQ(0, NumDiagnostics);
+}
+
+TEST_F(VFSFromYAMLTest, ReturnsInternalPathVFSHit) {
+  IntrusiveRefCntPtr BaseFS(
+  new vfs::InMemoryFileSystem);
+  BaseFS->addFile("//root/foo/realname", 0,
+  MemoryBuffer::getMemBuffer("contents of a"));
+  auto FS =
+  getFromYAMLString("{ 'use-external-names': false,\n"
+"  'roots': [\n"
+"{\n"
+"  'type': 'directory',\n"
+"  'name': '//root/foo',\n"
+"  'contents': [ {\n"
+"  'type': 'file',\n"
+"  'name': 'vfsname',\n"
+"  'external-contents': 'realname'\n"
+"}\n"
+"  ]\n"
+"}]}",
+BaseFS);
+  ASSERT_FALSE(FS->setCurrentWorkingDirectory("//root/foo"));
+
+  auto OpenedF = FS->openFileForRead("vfsname");
+  ASSERT_FALSE(OpenedF.getError());
+  llvm::ErrorOr Name = (*OpenedF)->getName();
+  ASSERT_FALSE(Name.getError());
+  EXPECT_EQ("vfsname", Name.get());
+
+  auto OpenedS = (*OpenedF)->status();
+  ASSERT_FALSE(OpenedS.getError());
+  EXPECT_EQ("vfsname", OpenedS->getName());
+  EXPECT_TRUE(OpenedS->IsVFSMapped);
+
+  auto DirectS = FS->status("vfsname");
+  ASSERT_FALSE(DirectS.getError());
+  EXPECT_EQ("vfsname", DirectS->getName());
+  EXPECT_TRUE(DirectS->IsVFSMapped);
+
+  EXPECT_EQ(0, NumDiagnostics);
+}
+
 TEST_F(VFSFromYAMLTest, CaseInsensitive) {
   IntrusiveRefCntPtr Lower(new DummyFileSystem());
   Lower->addRegularFile("//root/foo/bar/a");
Index: llvm/lib/Support/VirtualFileSystem.cpp
===
--- 

[PATCH] D113828: [clang-tidy] Fix false positives in `fuchsia-trailing-return` check involving deduction guides

[Fabian Wolff via Phabricator via cfe-commits]

fwolff created this revision.
fwolff added reviewers: alexfh, aaron.ballman, mizvekov.
fwolff added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, abrachet, phosek, xazax.hun.
fwolff requested review of this revision.
Herald added a subscriber: cfe-commits.

Fixes PR#47614. Deduction guides, implicit or user-defined, look like function 
declarations in the AST. They aren't really functions, though, and they always 
have a trailing return type, so it doesn't make sense to issue this warning for 
them.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113828

Files:
  clang-tools-extra/clang-tidy/fuchsia/TrailingReturnCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/fuchsia-trailing-return.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/fuchsia-trailing-return.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/fuchsia-trailing-return.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/fuchsia-trailing-return.cpp
@@ -1,9 +1,9 @@
-// RUN: %check_clang_tidy %s fuchsia-trailing-return %t
+// RUN: %check_clang_tidy -std=c++17-or-later %s fuchsia-trailing-return %t
 
 int add_one(const int arg) { return arg; }
 
 auto get_add_one() -> int (*)(const int) {
-  // CHECK-MESSAGES: [[@LINE-1]]:1: warning: a trailing return type is 
disallowed for this type of declaration
+  // CHECK-MESSAGES: [[@LINE-1]]:1: warning: a trailing return type is 
disallowed for this function declaration
   // CHECK-NEXT: auto get_add_one() -> int (*)(const int) {
   return add_one;
 }
@@ -21,3 +21,31 @@
 auto fn(const T1 , const T2 ) -> decltype(lhs + rhs) {
   return lhs + rhs;
 }
+
+// Now check that implicit and explicit C++17 deduction guides don't trigger 
this warning (PR#47614).
+
+template 
+struct ImplicitDeductionGuides {
+  ImplicitDeductionGuides(const T &);
+};
+
+template 
+struct pair {
+  A first;
+  B second;
+};
+
+template 
+struct UserDefinedDeductionGuides {
+  UserDefinedDeductionGuides(T);
+  template 
+  UserDefinedDeductionGuides(T1, T2);
+};
+
+template 
+UserDefinedDeductionGuides(T1, T2) -> UserDefinedDeductionGuides>;
+
+void foo() {
+  ImplicitDeductionGuides X(42);
+  UserDefinedDeductionGuides s(1, "abc");
+}
Index: clang-tools-extra/clang-tidy/fuchsia/TrailingReturnCheck.cpp
===
--- clang-tools-extra/clang-tidy/fuchsia/TrailingReturnCheck.cpp
+++ clang-tools-extra/clang-tidy/fuchsia/TrailingReturnCheck.cpp
@@ -17,12 +17,6 @@
 namespace tidy {
 namespace fuchsia {
 
-namespace {
-AST_MATCHER(FunctionDecl, hasTrailingReturn) {
-  return Node.getType()->castAs()->hasTrailingReturn();
-}
-} // namespace
-
 void TrailingReturnCheck::registerMatchers(MatchFinder *Finder) {
   // Functions that have trailing returns are disallowed, except for those
   // using decltype specifiers and lambda with otherwise unutterable
@@ -30,15 +24,16 @@
   Finder->addMatcher(
   functionDecl(hasTrailingReturn(),
unless(anyOf(returns(decltypeType()),
-hasParent(cxxRecordDecl(isLambda())
+hasParent(cxxRecordDecl(isLambda())),
+cxxDeductionGuideDecl(
   .bind("decl"),
   this);
 }
 
 void TrailingReturnCheck::check(const MatchFinder::MatchResult ) {
-  if (const auto *D = Result.Nodes.getNodeAs("decl"))
+  if (const auto *D = Result.Nodes.getNodeAs("decl"))
 diag(D->getBeginLoc(),
- "a trailing return type is disallowed for this type of declaration");
+ "a trailing return type is disallowed for this function declaration");
 }
 
 } // namespace fuchsia


Index: clang-tools-extra/test/clang-tidy/checkers/fuchsia-trailing-return.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/fuchsia-trailing-return.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/fuchsia-trailing-return.cpp
@@ -1,9 +1,9 @@
-// RUN: %check_clang_tidy %s fuchsia-trailing-return %t
+// RUN: %check_clang_tidy -std=c++17-or-later %s fuchsia-trailing-return %t
 
 int add_one(const int arg) { return arg; }
 
 auto get_add_one() -> int (*)(const int) {
-  // CHECK-MESSAGES: [[@LINE-1]]:1: warning: a trailing return type is disallowed for this type of declaration
+  // CHECK-MESSAGES: [[@LINE-1]]:1: warning: a trailing return type is disallowed for this function declaration
   // CHECK-NEXT: auto get_add_one() -> int (*)(const int) {
   return add_one;
 }
@@ -21,3 +21,31 @@
 auto fn(const T1 , const T2 ) -> decltype(lhs + rhs) {
   return lhs + rhs;
 }
+
+// Now check that implicit and explicit C++17 deduction guides don't trigger this warning (PR#47614).
+
+template 
+struct ImplicitDeductionGuides {
+  ImplicitDeductionGuides(const T &);
+};
+
+template 
+struct pair {
+  A first;
+  B second;
+};
+
+template 
+struct UserDefinedDeductionGuides {
+  

[PATCH] D113826: [clang-format][c++2b] support removal of the space between auto and {} in P0849R8

[MyDeveloperDay via Phabricator via cfe-commits]

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

Revert the last change but comment that the first 4 tests are from the unit 
tests of D113393  which are expected invalid 
(I wouldn't want the unit tests from formatting incorrectly)


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

https://reviews.llvm.org/D113826

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -22566,6 +22566,29 @@
   EXPECT_EQ(Code, format(Code, Style));
 }
 
+TEST_F(FormatTest, FormatDecayCopy) {
+  // error cases from unit tests
+  verifyFormat("foo(auto())");
+  verifyFormat("foo(auto{})");
+  verifyFormat("foo(auto({}))");
+  verifyFormat("foo(auto{{}})");
+
+  verifyFormat("foo(auto(1))");
+  verifyFormat("foo(auto{1})");
+  verifyFormat("foo(new auto(1))");
+  verifyFormat("foo(new auto{1})");
+  verifyFormat("decltype(auto(1)) x;");
+  verifyFormat("decltype(auto{1}) x;");
+  verifyFormat("auto(x);");
+  verifyFormat("auto{x};");
+  verifyFormat("auto{x} = y;");
+  verifyFormat("auto(x) = y;"); // actually a declaration, but this is clearly
+// the user's own fault
+  verifyFormat("integral auto(x) = y;"); // actually a declaration, but this is
+ // clearly the user's own fault
+  verifyFormat("auto(*p)() = f;");   // actually a declaration; TODO FIXME
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2940,6 +2940,10 @@
   return true;
   }
 
+  // auto{x}
+  if (Left.is(tok::kw_auto) && Right.isOneOf(tok::l_paren, tok::l_brace))
+return false;
+
   // requires clause Concept1 && Concept2
   if (Left.is(TT_ConstraintJunctions) && Right.is(tok::identifier))
 return true;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -22566,6 +22566,29 @@
   EXPECT_EQ(Code, format(Code, Style));
 }
 
+TEST_F(FormatTest, FormatDecayCopy) {
+  // error cases from unit tests
+  verifyFormat("foo(auto())");
+  verifyFormat("foo(auto{})");
+  verifyFormat("foo(auto({}))");
+  verifyFormat("foo(auto{{}})");
+
+  verifyFormat("foo(auto(1))");
+  verifyFormat("foo(auto{1})");
+  verifyFormat("foo(new auto(1))");
+  verifyFormat("foo(new auto{1})");
+  verifyFormat("decltype(auto(1)) x;");
+  verifyFormat("decltype(auto{1}) x;");
+  verifyFormat("auto(x);");
+  verifyFormat("auto{x};");
+  verifyFormat("auto{x} = y;");
+  verifyFormat("auto(x) = y;"); // actually a declaration, but this is clearly
+// the user's own fault
+  verifyFormat("integral auto(x) = y;"); // actually a declaration, but this is
+ // clearly the user's own fault
+  verifyFormat("auto(*p)() = f;");   // actually a declaration; TODO FIXME
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2940,6 +2940,10 @@
   return true;
   }
 
+  // auto{x}
+  if (Left.is(tok::kw_auto) && Right.isOneOf(tok::l_paren, tok::l_brace))
+return false;
+
   // requires clause Concept1 && Concept2
   if (Left.is(TT_ConstraintJunctions) && Right.is(tok::identifier))
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113826: [clang-format][c++2b] support removal of the space between auto and {} in P0849R8

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Sorry my mistake, the 1st 4 I want to keep because they related to the unit 
tests for D113393: [c++2b] Implement P0849R8 auto(x) 


  foo(auto());   // expected-error {{initializer for functional-style cast to 
'auto' is empty}}
  foo(auto{});   // expected-error {{initializer for functional-style cast to 
'auto' is empty}}
  foo(auto({})); // expected-error {{cannot deduce actual type for 'auto' from 
parenthesized initializer list}}
  foo(auto{{}}); // expected-error {{cannot deduce actual type for 'auto' from 
nested initializer list}}


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

https://reviews.llvm.org/D113826

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

[PATCH] D110745: Redefine deref(N) attribute family as point-in-time semantics (aka deref-at-point)

[Nikita Popov via Phabricator via cfe-commits]

nikic added a subscriber: aeubanks.
nikic added a comment.

In D110745#3128719 , @reames wrote:

> @nikic ping on previous question.  It's been a month, and this has been 
> LGTMed.  Without response, I plan to land this.

Sorry, I did do some measurements but forgot to report back. The only run-time 
workload I can easily measure are rustc check builds, where I observed 
regressions ranging between -0.6% and +4.8% across different projects (the 
-0.6% "regressions" indicate an improvement, i.e. that some deref-based 
optimization happens to be non-profitable). I'm not sure that helps you much 
apart from saying that yes indeed, this does have some impact on rust code. 
It's not catastrophic (though caveat emptor: impact on "benchmarky" code may 
well be higher), but it's also avoidable.

> In D110745#3038848 , @xbolva00 
> wrote:
>
>> This really needs to be properly benchmarked.
>
> This has been benchmarked on every workload I care about, and shows no 
> interesting regressions.   Unfortunately, those are all non-public Java 
> workloads,

I think something important to mention here is that the Java (i.e. GC'd 
language) case is also the only where we don't really expect a regression, 
because it has good modeling under the proposed patch: GC'd pointers can't be 
freed during the optimization pipeline (until statepoint lowering), so they're 
simply not affected by this change. For that reason I don't think the fact that 
Java workloads don't see regressions really tells us anything about how this 
would behave for other frontends, which are mostly not GC'd.

> On the C/C++ side, I don't have a ready environment in which to run anything 
> representative.  From the semantic change, I wouldn't expect C++ to show much 
> difference, and besides, this is fixing a long standing fairly major 
> correctness issue.  If you have particular suites you care about, please run 
> them and share results.

Maybe @asbirlea or @aeubanks can run some benchmarks? I would expect 
regressions for C++, because `nofree` inference is very limited (e.g. won't be 
inferred for pretty much any code using growing STL containers). Though at 
least in the C++ case regressions may be somewhat justified, in that this fixes 
a correctness issue in some cases.

> At this point, I strongly lean towards committing and letting regressions be 
> reported.  We might revert, or we might simply fix forward depending on what 
> comes up

I'm not sure what you're expecting from that. At least as far as Rust is 
concerned, the problem here seems pretty well-understood to me: You are 
dropping (without replacement) the ability to specify that an argument is 
dereferenceable within a function. I'm perfectly happy with the change in 
default behavior, all I want is a way to get the old one back. I don't think 
that having an example of that in "real" code is going to add any useful 
information.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110745

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

[PATCH] D113826: [clang-format][c++2b] support removal of the space between auto and {} in P0849R8

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

arthur.j.odwyer added subscribers: MyDeveloperDay, arthur.j.odwyer.
arthur.j.odwyer added a comment.

Sorry, I meant remove the first four cases that are invalid C++, e.g.
auto{}.
Personally I'd keep the "valid but arguably misformatted" cases (which I
had suggested earlier but you just removed).
How that's clearer now.


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

https://reviews.llvm.org/D113826

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

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

[Cameron Mulhern via Phabricator via cfe-commits]

csmulhern marked 4 inline comments as done.
csmulhern added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:951
 
+  if (PreviousNonComment && PreviousNonComment->is(tok::l_paren)) {
+State.Stack.back().BreakBeforeClosingParen =

owenpan wrote:
> HazardyKnusperkeks wrote:
> > Remove the braces
> I think the braces should not be removed here. The [[ 
> https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
>  | LLVM Coding Standards ]] says "braces should be used when a 
> single-statement body is complex enough" which is rather subjective, but line 
> wrapping is an objective metric and perhaps the only one we can apply.
> 
> 
> 
I'm not sure either way, but the prevailing style in this file seems like it 
would support having no braces, so I went ahead and made the change.



Comment at: clang/unittests/Format/FormatTest.cpp:22453
+  Style.AllowAllArgumentsOnNextLine = false;
+  Style.AllowAllConstructorInitializersOnNextLine = false;
+  Style.AllowAllParametersOfDeclarationOnNextLine = false;

HazardyKnusperkeks wrote:
> How does it behave without these?
It should put the closing parenthesis on a newline as long as it was opened 
with a newline. I've added additional test cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109557

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

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

[Cameron Mulhern via Phabricator via cfe-commits]

csmulhern updated this revision to Diff 387025.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109557

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -18706,6 +18706,8 @@
   FormatStyle::BAS_DontAlign);
   CHECK_PARSE("AlignAfterOpenBracket: AlwaysBreak", AlignAfterOpenBracket,
   FormatStyle::BAS_AlwaysBreak);
+  CHECK_PARSE("AlignAfterOpenBracket: BlockIndent", AlignAfterOpenBracket,
+  FormatStyle::BAS_BlockIndent);
   // For backward compatibility:
   CHECK_PARSE("AlignAfterOpenBracket: false", AlignAfterOpenBracket,
   FormatStyle::BAS_DontAlign);
@@ -22566,6 +22568,224 @@
   EXPECT_EQ(Code, format(Code, Style));
 }
 
+TEST_F(FormatTest, AlignAfterOpenBracketBlockIndent) {
+  auto Style = getLLVMStyle();
+
+  StringRef Short = "functionCall(paramA, paramB, paramC);\n"
+"void functionDecl(int a, int b, int c);";
+
+  StringRef Medium = "functionCall(paramA, paramB, paramC, paramD, paramE, "
+ "paramF, paramG, paramH, paramI);\n"
+ "void functionDecl(int argumentA, int argumentB, int "
+ "argumentC, int argumentD, int argumentE);";
+
+  verifyFormat(Short, Style);
+
+  StringRef NoBreak = "functionCall(paramA, paramB, paramC, paramD, paramE, "
+  "paramF, paramG, paramH,\n"
+  " paramI);\n"
+  "void functionDecl(int argumentA, int argumentB, int "
+  "argumentC, int argumentD,\n"
+  "  int argumentE);";
+
+  verifyFormat(NoBreak, Medium, Style);
+  verifyFormat(NoBreak,
+   "functionCall(\n"
+   "paramA,\n"
+   "paramB,\n"
+   "paramC,\n"
+   "paramD,\n"
+   "paramE,\n"
+   "paramF,\n"
+   "paramG,\n"
+   "paramH,\n"
+   "paramI\n"
+   ");\n"
+   "void functionDecl(\n"
+   "int argumentA,\n"
+   "int argumentB,\n"
+   "int argumentC,\n"
+   "int argumentD,\n"
+   "int argumentE\n"
+   ");",
+   Style);
+
+  verifyFormat("outerFunctionCall(nestedFunctionCall(argument1),\n"
+   "  nestedLongFunctionCall(argument1, "
+   "argument2, argument3,\n"
+   " argument4, "
+   "argument5));",
+   Style);
+
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_BlockIndent;
+
+  verifyFormat(Short, Style);
+  verifyFormat(
+  "functionCall(\n"
+  "paramA, paramB, paramC, paramD, paramE, paramF, paramG, paramH, "
+  "paramI\n"
+  ");\n"
+  "void functionDecl(\n"
+  "int argumentA, int argumentB, int argumentC, int argumentD, int "
+  "argumentE\n"
+  ");",
+  Medium, Style);
+
+  Style.AllowAllArgumentsOnNextLine = false;
+  Style.AllowAllParametersOfDeclarationOnNextLine = false;
+
+  verifyFormat(Short, Style);
+  verifyFormat(
+  "functionCall(\n"
+  "paramA, paramB, paramC, paramD, paramE, paramF, paramG, paramH, "
+  "paramI\n"
+  ");\n"
+  "void functionDecl(\n"
+  "int argumentA, int argumentB, int argumentC, int argumentD, int "
+  "argumentE\n"
+  ");",
+  Medium, Style);
+
+  Style.BinPackArguments = false;
+  Style.BinPackParameters = false;
+
+  verifyFormat(Short, Style);
+
+  verifyFormat("functionCall(\n"
+   "paramA,\n"
+   "paramB,\n"
+   "paramC,\n"
+   "paramD,\n"
+   "paramE,\n"
+   "paramF,\n"
+   "paramG,\n"
+   "paramH,\n"
+   "paramI\n"
+   ");\n"
+   "void functionDecl(\n"
+   "int argumentA,\n"
+   "int argumentB,\n"
+   "int argumentC,\n"
+   "int argumentD,\n"
+   "int argumentE\n"
+   ");",
+   Medium, Style);
+
+  verifyFormat("outerFunctionCall(\n"
+   "nestedFunctionCall(argument1),\n"
+   "nestedLongFunctionCall(\n"
+   "argument1,\n"
+   "argument2,\n"
+   "

[PATCH] D113826: [clang-format][c++2b] support removal of the space between auto and {} in P0849R8

[MyDeveloperDay via Phabricator via cfe-commits]

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

Remove invalid testcases


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

https://reviews.llvm.org/D113826

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -22566,6 +22566,22 @@
   EXPECT_EQ(Code, format(Code, Style));
 }
 
+TEST_F(FormatTest, FormatDecayCopy) {
+  verifyFormat("foo(auto())");
+  verifyFormat("foo(auto{})");
+  verifyFormat("foo(auto({}))");
+  verifyFormat("foo(auto{{}})");
+  verifyFormat("foo(auto(1))");
+  verifyFormat("foo(auto{1})");
+  verifyFormat("foo(new auto(1))");
+  verifyFormat("foo(new auto{1})");
+  verifyFormat("decltype(auto(1)) x;");
+  verifyFormat("decltype(auto{1}) x;");
+  verifyFormat("auto(x);");
+  verifyFormat("auto{x};");
+  verifyFormat("auto{x} = y;");
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2940,6 +2940,10 @@
   return true;
   }
 
+  // auto{x}
+  if (Left.is(tok::kw_auto) && Right.isOneOf(tok::l_paren, tok::l_brace))
+return false;
+
   // requires clause Concept1 && Concept2
   if (Left.is(TT_ConstraintJunctions) && Right.is(tok::identifier))
 return true;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -22566,6 +22566,22 @@
   EXPECT_EQ(Code, format(Code, Style));
 }
 
+TEST_F(FormatTest, FormatDecayCopy) {
+  verifyFormat("foo(auto())");
+  verifyFormat("foo(auto{})");
+  verifyFormat("foo(auto({}))");
+  verifyFormat("foo(auto{{}})");
+  verifyFormat("foo(auto(1))");
+  verifyFormat("foo(auto{1})");
+  verifyFormat("foo(new auto(1))");
+  verifyFormat("foo(new auto{1})");
+  verifyFormat("decltype(auto(1)) x;");
+  verifyFormat("decltype(auto{1}) x;");
+  verifyFormat("auto(x);");
+  verifyFormat("auto{x};");
+  verifyFormat("auto{x} = y;");
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2940,6 +2940,10 @@
   return true;
   }
 
+  // auto{x}
+  if (Left.is(tok::kw_auto) && Right.isOneOf(tok::l_paren, tok::l_brace))
+return false;
+
   // requires clause Concept1 && Concept2
   if (Left.is(TT_ConstraintJunctions) && Right.is(tok::identifier))
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113826: [clang-format][c++2b] support removal of the space between auto and {} in P0849R8

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

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

Personally I'd remove the tests that are invalid expressions; but I don't know 
what's the usual practice. Ship it AFAIC!


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

https://reviews.llvm.org/D113826

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

[PATCH] D113826: [clang-format][c++2b] support removal of the space between auto and {} in P0849R8

[MyDeveloperDay via Phabricator via cfe-commits]

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

Additional use cases


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

https://reviews.llvm.org/D113826

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -22566,6 +22566,27 @@
   EXPECT_EQ(Code, format(Code, Style));
 }
 
+TEST_F(FormatTest, FormatDecayCopy) {
+  verifyFormat("foo(auto())");
+  verifyFormat("foo(auto{})");
+  verifyFormat("foo(auto({}))");
+  verifyFormat("foo(auto{{}})");
+  verifyFormat("foo(auto(1))");
+  verifyFormat("foo(auto{1})");
+  verifyFormat("foo(new auto(1))");
+  verifyFormat("foo(new auto{1})");
+  verifyFormat("decltype(auto(1)) x;");
+  verifyFormat("decltype(auto{1}) x;");
+  verifyFormat("auto(x);");
+  verifyFormat("auto{x};");
+  verifyFormat("auto{x} = y;");
+  verifyFormat("auto(x) = y;"); // actually a declaration, but this is clearly
+// the user's own fault
+  verifyFormat("integral auto(x) = y;"); // actually a declaration, but this is
+ // clearly the user's own fault
+  verifyFormat("auto(*p)() = f;");   // actually a declaration; TODO FIXME
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2940,6 +2940,10 @@
   return true;
   }
 
+  // auto{x}
+  if (Left.is(tok::kw_auto) && Right.isOneOf(tok::l_paren, tok::l_brace))
+return false;
+
   // requires clause Concept1 && Concept2
   if (Left.is(TT_ConstraintJunctions) && Right.is(tok::identifier))
 return true;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -22566,6 +22566,27 @@
   EXPECT_EQ(Code, format(Code, Style));
 }
 
+TEST_F(FormatTest, FormatDecayCopy) {
+  verifyFormat("foo(auto())");
+  verifyFormat("foo(auto{})");
+  verifyFormat("foo(auto({}))");
+  verifyFormat("foo(auto{{}})");
+  verifyFormat("foo(auto(1))");
+  verifyFormat("foo(auto{1})");
+  verifyFormat("foo(new auto(1))");
+  verifyFormat("foo(new auto{1})");
+  verifyFormat("decltype(auto(1)) x;");
+  verifyFormat("decltype(auto{1}) x;");
+  verifyFormat("auto(x);");
+  verifyFormat("auto{x};");
+  verifyFormat("auto{x} = y;");
+  verifyFormat("auto(x) = y;"); // actually a declaration, but this is clearly
+// the user's own fault
+  verifyFormat("integral auto(x) = y;"); // actually a declaration, but this is
+ // clearly the user's own fault
+  verifyFormat("auto(*p)() = f;");   // actually a declaration; TODO FIXME
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2940,6 +2940,10 @@
   return true;
   }
 
+  // auto{x}
+  if (Left.is(tok::kw_auto) && Right.isOneOf(tok::l_paren, tok::l_brace))
+return false;
+
   // requires clause Concept1 && Concept2
   if (Left.is(TT_ConstraintJunctions) && Right.is(tok::identifier))
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113826: [clang-format][c++2b] support removal of the space between auto and {} in P0849R8

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

Quuxplusone requested changes to this revision.
Quuxplusone added inline comments.
This revision now requires changes to proceed.



Comment at: clang/unittests/Format/FormatTest.cpp:22574
+  verifyFormat("foo(auto{{}})");
+}
+

Looks like a good improvement to me!
However, note that `auto()`, `auto{}`, `auto({})`, `auto{{}}` are all invalid 
expressions; and it would be good to test with something inside the braces 
anyway. Please replace with e.g.
```
verifyFormat("foo(auto(1))");
verifyFormat("foo(auto{1})");
verifyFormat("foo(new auto(1))");
verifyFormat("foo(new auto{1})");
verifyFormat("decltype(auto(1)) x;");
verifyFormat("decltype(auto{1}) x;");
verifyFormat("auto(x);");
verifyFormat("auto{x};");
verifyFormat("auto{x} = y;");
verifyFormat("auto(x) = y;");  // actually a declaration, but this is clearly 
the user's own fault
verifyFormat("integral auto(x) = y;");  // actually a declaration, but this is 
clearly the user's own fault
verifyFormat("auto(*p)() = f;");  // actually a declaration; TODO FIXME
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113826

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

[PATCH] D113826: [clang-format][c++2b] support removal of the space between auto and {} in P0849R8

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: lichray, HazardyKnusperkeks, curdeius.
MyDeveloperDay added projects: clang, clang-format.
MyDeveloperDay requested review of this revision.

Looks like the work of D113393: [c++2b] Implement P0849R8 auto(x) 
 requires manual clang-formatting 
intervention.

Removal of the space between `auto` and `{}`

@lichray would this help?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113826

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -22566,6 +22566,13 @@
   EXPECT_EQ(Code, format(Code, Style));
 }
 
+TEST_F(FormatTest, FormatDecayCopy) {
+  verifyFormat("foo(auto())");
+  verifyFormat("foo(auto{})");
+  verifyFormat("foo(auto({}))");
+  verifyFormat("foo(auto{{}})");
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2940,6 +2940,10 @@
   return true;
   }
 
+  // auto{x}
+  if (Left.is(tok::kw_auto) && Right.is(tok::l_brace))
+return false;
+
   // requires clause Concept1 && Concept2
   if (Left.is(TT_ConstraintJunctions) && Right.is(tok::identifier))
 return true;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -22566,6 +22566,13 @@
   EXPECT_EQ(Code, format(Code, Style));
 }
 
+TEST_F(FormatTest, FormatDecayCopy) {
+  verifyFormat("foo(auto())");
+  verifyFormat("foo(auto{})");
+  verifyFormat("foo(auto({}))");
+  verifyFormat("foo(auto{{}})");
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2940,6 +2940,10 @@
   return true;
   }
 
+  // auto{x}
+  if (Left.is(tok::kw_auto) && Right.is(tok::l_brace))
+return false;
+
   // requires clause Concept1 && Concept2
   if (Left.is(TT_ConstraintJunctions) && Right.is(tok::identifier))
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 6e58d14 - [clang-format] [PR52228] clang-format csharp inconsistant nested namespace indentation

[via cfe-commits]

Author: mydeveloperday
Date: 2021-11-13T14:13:51Z
New Revision: 6e58d14e5b012215007ae375006f5e847a9b61bc

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

LOG: [clang-format] [PR52228] clang-format csharp inconsistant nested namespace 
indentation

https://bugs.llvm.org/show_bug.cgi?id=52228

For multilevel namespaces in C# get their content indented when 
NamespaceIndentation: None is set, where as single level namespaces are 
formatted correctly.

Reviewed By: HazardyKnusperkeks, jbcoe

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

Added: 


Modified: 
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTestCSharp.cpp

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index ae38e387f1df2..5ed25f6e27982 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -2216,7 +2216,7 @@ void UnwrappedLineParser::parseNamespace() {
 parseParens();
   } else {
 while (FormatTok->isOneOf(tok::identifier, tok::coloncolon, tok::kw_inline,
-  tok::l_square)) {
+  tok::l_square, tok::period)) {
   if (FormatTok->is(tok::l_square))
 parseSquare();
   else

diff  --git a/clang/unittests/Format/FormatTestCSharp.cpp 
b/clang/unittests/Format/FormatTestCSharp.cpp
index b7ea8d3672a2a..bb91cd6cf2b06 100644
--- a/clang/unittests/Format/FormatTestCSharp.cpp
+++ b/clang/unittests/Format/FormatTestCSharp.cpp
@@ -1314,5 +1314,60 @@ TEST_F(FormatTestCSharp, CSharpAfterClass) {
Style);
 }
 
+TEST_F(FormatTestCSharp, NamespaceIndentation) {
+  FormatStyle Style = getMicrosoftStyle(FormatStyle::LK_CSharp);
+  Style.NamespaceIndentation = FormatStyle::NI_None;
+
+  verifyFormat("namespace A\n"
+   "{\n"
+   "public interface Name1\n"
+   "{\n"
+   "}\n"
+   "}\n",
+   Style);
+
+  verifyFormat("namespace A.B\n"
+   "{\n"
+   "public interface Name1\n"
+   "{\n"
+   "}\n"
+   "}\n",
+   Style);
+
+  Style.NamespaceIndentation = FormatStyle::NI_Inner;
+
+  verifyFormat("namespace A\n"
+   "{\n"
+   "namespace B\n"
+   "{\n"
+   "public interface Name1\n"
+   "{\n"
+   "}\n"
+   "}\n"
+   "}\n",
+   Style);
+
+  Style.NamespaceIndentation = FormatStyle::NI_All;
+
+  verifyFormat("namespace A.B\n"
+   "{\n"
+   "public interface Name1\n"
+   "{\n"
+   "}\n"
+   "}\n",
+   Style);
+
+  verifyFormat("namespace A\n"
+   "{\n"
+   "namespace B\n"
+   "{\n"
+   "public interface Name1\n"
+   "{\n"
+   "}\n"
+   "}\n"
+   "}\n",
+   Style);
+}
+
 } // namespace format
 } // end namespace clang



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

[PATCH] D112887: [clang-format] [PR52228] clang-format csharp inconsistant nested namespace indentation

[MyDeveloperDay 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 rG6e58d14e5b01: [clang-format] [PR52228] clang-format csharp 
inconsistant nested namespace… (authored by MyDeveloperDay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112887

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTestCSharp.cpp


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -1314,5 +1314,60 @@
Style);
 }
 
+TEST_F(FormatTestCSharp, NamespaceIndentation) {
+  FormatStyle Style = getMicrosoftStyle(FormatStyle::LK_CSharp);
+  Style.NamespaceIndentation = FormatStyle::NI_None;
+
+  verifyFormat("namespace A\n"
+   "{\n"
+   "public interface Name1\n"
+   "{\n"
+   "}\n"
+   "}\n",
+   Style);
+
+  verifyFormat("namespace A.B\n"
+   "{\n"
+   "public interface Name1\n"
+   "{\n"
+   "}\n"
+   "}\n",
+   Style);
+
+  Style.NamespaceIndentation = FormatStyle::NI_Inner;
+
+  verifyFormat("namespace A\n"
+   "{\n"
+   "namespace B\n"
+   "{\n"
+   "public interface Name1\n"
+   "{\n"
+   "}\n"
+   "}\n"
+   "}\n",
+   Style);
+
+  Style.NamespaceIndentation = FormatStyle::NI_All;
+
+  verifyFormat("namespace A.B\n"
+   "{\n"
+   "public interface Name1\n"
+   "{\n"
+   "}\n"
+   "}\n",
+   Style);
+
+  verifyFormat("namespace A\n"
+   "{\n"
+   "namespace B\n"
+   "{\n"
+   "public interface Name1\n"
+   "{\n"
+   "}\n"
+   "}\n"
+   "}\n",
+   Style);
+}
+
 } // namespace format
 } // end namespace clang
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2216,7 +2216,7 @@
 parseParens();
   } else {
 while (FormatTok->isOneOf(tok::identifier, tok::coloncolon, tok::kw_inline,
-  tok::l_square)) {
+  tok::l_square, tok::period)) {
   if (FormatTok->is(tok::l_square))
 parseSquare();
   else


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -1314,5 +1314,60 @@
Style);
 }
 
+TEST_F(FormatTestCSharp, NamespaceIndentation) {
+  FormatStyle Style = getMicrosoftStyle(FormatStyle::LK_CSharp);
+  Style.NamespaceIndentation = FormatStyle::NI_None;
+
+  verifyFormat("namespace A\n"
+   "{\n"
+   "public interface Name1\n"
+   "{\n"
+   "}\n"
+   "}\n",
+   Style);
+
+  verifyFormat("namespace A.B\n"
+   "{\n"
+   "public interface Name1\n"
+   "{\n"
+   "}\n"
+   "}\n",
+   Style);
+
+  Style.NamespaceIndentation = FormatStyle::NI_Inner;
+
+  verifyFormat("namespace A\n"
+   "{\n"
+   "namespace B\n"
+   "{\n"
+   "public interface Name1\n"
+   "{\n"
+   "}\n"
+   "}\n"
+   "}\n",
+   Style);
+
+  Style.NamespaceIndentation = FormatStyle::NI_All;
+
+  verifyFormat("namespace A.B\n"
+   "{\n"
+   "public interface Name1\n"
+   "{\n"
+   "}\n"
+   "}\n",
+   Style);
+
+  verifyFormat("namespace A\n"
+   "{\n"
+   "namespace B\n"
+   "{\n"
+   "public interface Name1\n"
+   "{\n"
+   "}\n"
+   "}\n"
+   "}\n",
+   Style);
+}
+
 } // namespace format
 } // end namespace clang
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2216,7 +2216,7 @@
 parseParens();
   } else {
 while (FormatTok->isOneOf(tok::identifier, tok::coloncolon, tok::kw_inline,
-  tok::l_square)) {
+  

[PATCH] D113393: [c++2b] Implement P0849R8 auto(x)

[Zhihao Yuan via Phabricator via cfe-commits]

lichray updated this revision to Diff 387017.
lichray added a comment.

- Manually adjust auto{x} formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113393

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p5.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.auto.deduct/p2.cpp
  clang/test/CXX/expr/expr.post/expr.type.conv/p1-2b.cpp
  clang/test/CXX/expr/expr.unary/expr.new/p2-cxx0x.cpp
  clang/test/CXX/expr/expr.unary/expr.new/p2-cxx14.cpp
  clang/test/CXX/expr/expr.unary/expr.new/p2-cxx1z.cpp
  clang/test/Parser/cxx2b-auto-x.cpp
  clang/test/SemaCXX/deduced-return-type-cxx14.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1379,6 +1379,11 @@
   https://wg21.link/P2360R0;>P2360R0
   Clang 14
 
+
+  auto(x): decay-copy in the language
+  https://wg21.link/P0849R8;>P0849R8
+  Clang 14
+
 
 
 
Index: clang/test/SemaCXX/deduced-return-type-cxx14.cpp
===
--- clang/test/SemaCXX/deduced-return-type-cxx14.cpp
+++ clang/test/SemaCXX/deduced-return-type-cxx14.cpp
@@ -638,5 +638,7 @@
   using B = auto (*)() -> auto; // expected-error {{'auto' not allowed in type alias}}
   template auto> struct X {}; // cxx14-error {{'auto' not allowed in template parameter until C++17}}
   template struct Y { T x; };
-  Y auto> y; // expected-error {{'auto' not allowed in template argument}}
+  Y auto> y; // cxx14_20-error {{'auto' not allowed in template argument}} \
+  cxx2b-error {{initializer for functional-style cast to 'auto' is empty}} \
+  cxx2b-error {{expected unqualified-id}}
 }
Index: clang/test/Parser/cxx2b-auto-x.cpp
===
--- /dev/null
+++ clang/test/Parser/cxx2b-auto-x.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx2b -std=c++2b %s
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx20 -std=c++20 %s
+
+void looks_like_decltype_auto() {
+  decltype(auto(42)) b = 42; // cxx20-error {{'auto' not allowed here}}
+  decltype(long *) a = 42;   // expected-error {{expected '(' for function-style cast or type construction}} \
+expected-error {{expected expression}}
+  decltype(auto *) a = 42;   // expected-error {{expected '(' for function-style cast or type construction}} \
+expected-error {{expected expression}}
+  decltype(auto()) c = 42;   // cxx2b-error {{initializer for functional-style cast to 'auto' is empty}} \
+cxx20-error {{'auto' not allowed here}}
+}
+
+struct looks_like_declaration {
+  int n;
+} a;
+
+using T = looks_like_declaration *;
+void f() { T()->n = 1; }
+void g() { auto()->n = 0; } // cxx20-error {{declaration of variable 'a' with deduced type 'auto (&)' requires an initializer}} \
+ cxx20-error {{expected ';' at end of declaration}}
+void h() { auto{}->n = 0; } // cxx20-error {{expected unqualified-id}} \
+ cxx20-error {{expected expression}}
Index: clang/test/CXX/expr/expr.unary/expr.new/p2-cxx1z.cpp
===
--- clang/test/CXX/expr/expr.unary/expr.new/p2-cxx1z.cpp
+++ clang/test/CXX/expr/expr.unary/expr.new/p2-cxx1z.cpp
@@ -1,11 +1,30 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++14
 // RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++17 -pedantic
 
+// [expr.new]p2 ... the invented declaration: T x init ;
+// C++2b [dcl.type.auto.deduct]p2.2
+// For a variable declared with a type that contains a placeholder type, T is the declared type of the variable.
 void f() {
+  // - If the initializer is a parenthesized expression-list, the expression-list shall be a single assignmentexpression and E is the assignment-expression.
   new auto('a');
-  new auto {2};
-  new auto {1, 2}; // expected-error{{new expression for type 'auto' contains multiple constructor arguments}}
-  new auto {}; // expected-error{{new expression for type 'auto' requires a constructor argument}}
-  new decltype(auto)({1});
-  new decltype(auto)({1, 2}); // expected-error{{new expression for type 'decltype(auto)' contains multiple constructor arguments}}
+  new decltype(auto)('a');
+  // - If the initializer is a braced-init-list, it shall consist of a single brace-enclosed assignment-expression and E is the assignment-expression.
+  new 

[PATCH] D95168: [clang-format] Add Insert/Remove Braces option

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay marked 8 inline comments as done.
MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/BraceInserter.cpp:21-56
+bool SupportsAnyAutomaticBraces(FormatStyle , bool insertion) {
+  if (insertion) {
+if (Style.AutomaticBraces.AfterIf == FormatStyle::BIS_Always)
+  return true;
+if (Style.AutomaticBraces.AfterIf == FormatStyle::BIS_WrapLikely)
+  return true;
+if (Style.AutomaticBraces.AfterFor == FormatStyle::BIS_Always)

HazardyKnusperkeks wrote:
> How about something like that?
I think for now

```
bool SupportsAnyAutomaticBraces(FormatStyle , bool insertion) {
  if (   Style.AutomaticBraces.AfterIf == FormatStyle::BIS_Leave
  && Style.AutomaticBraces.AfterFor == FormatStyle::BIS_Leave
  && Style.AutomaticBraces.AfterWhile == FormatStyle::BIS_Leave
  && Style.AutomaticBraces.AfterDo == FormatStyle::BIS_Leave
  && Style.AutomaticBraces.AfterElse == FormatStyle::BIS_Leave)
  return false;

  if (!insertion){
if (Style.AutomaticBraces.AfterIf != FormatStyle::BIS_Remove)
&& Style.AutomaticBraces.AfterFor != FormatStyle::BIS_Remove)
&& Style.AutomaticBraces.AfterWhile != FormatStyle::BIS_Remove)
&& Style.AutomaticBraces.AfterDo != FormatStyle::BIS_Remove)
&& Style.AutomaticBraces.AfterElse != FormatStyle::BIS_Remove)
  return false;
  }
  return true;
}
```

Should cover it.


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

https://reviews.llvm.org/D95168

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

[PATCH] D95168: [clang-format] Add Insert/Remove Braces option

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 387015.
MyDeveloperDay marked an inline comment as done.
MyDeveloperDay edited the summary of this revision.
MyDeveloperDay added a comment.

Add more LLVM rules, and don't remove when there are single or multi lines 
comments


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

https://reviews.llvm.org/D95168

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/AutomaticBraces.cpp
  clang/lib/Format/AutomaticBraces.h
  clang/lib/Format/CMakeLists.txt
  clang/lib/Format/Format.cpp
  clang/unittests/Format/AutomaticBracesTest.cpp
  clang/unittests/Format/CMakeLists.txt
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -72,6 +72,8 @@
 EXPECT_EQ(Expected.str(), format(Expected, Style))
 << "Expected code is not stable";
 EXPECT_EQ(Expected.str(), format(Code, Style));
+// clang::format::internal::reformat does not run any of the options that
+// modify code for ObjC
 if (Style.Language == FormatStyle::LK_Cpp) {
   // Objective-C++ is a superset of C++, so everything checked for C++
   // needs to be checked for Objective-C++ as well.
@@ -6530,9 +6532,8 @@
   OnePerLine.BinPackParameters = false;
   std::string input = "Constructor()\n"
   ": (a,\n";
-  for (unsigned i = 0, e = 80; i != e; ++i) {
+  for (unsigned i = 0, e = 80; i != e; ++i)
 input += "   a,\n";
-  }
   input += "   a) {}";
   verifyFormat(input, OnePerLine);
 }
@@ -22396,7 +22397,6 @@
   "}";
   EXPECT_EQ(Code, format(Code, Style));
 }
-
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/unittests/Format/CMakeLists.txt
===
--- clang/unittests/Format/CMakeLists.txt
+++ clang/unittests/Format/CMakeLists.txt
@@ -3,6 +3,7 @@
   )
 
 add_clang_unittest(FormatTests
+  AutomaticBracesTest.cpp
   CleanupTest.cpp
   FormatTest.cpp
   FormatTestComments.cpp
Index: clang/unittests/Format/AutomaticBracesTest.cpp
===
--- /dev/null
+++ clang/unittests/Format/AutomaticBracesTest.cpp
@@ -0,0 +1,810 @@
+//===- unittest/Format/AutomaticBracesTest.cpp - brace insertion unit tests
+//-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Format/Format.h"
+
+#include "../Tooling/ReplacementTest.h"
+#include "FormatTestUtils.h"
+
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "gtest/gtest.h"
+
+#define DEBUG_TYPE "automatic-braces-test"
+
+using clang::tooling::ReplacementTest;
+using clang::tooling::toReplacements;
+using testing::ScopedTrace;
+
+namespace clang {
+namespace format {
+namespace {
+
+class AutomaticBracesTest : public ::testing::Test {
+protected:
+  enum StatusCheck { SC_ExpectComplete, SC_ExpectIncomplete, SC_DoNotCheck };
+
+  std::string format(llvm::StringRef Code,
+ const FormatStyle  = getLLVMStyle(),
+ StatusCheck CheckComplete = SC_ExpectComplete) {
+LLVM_DEBUG(llvm::errs() << "---\n");
+LLVM_DEBUG(llvm::errs() << Code << "\n\n");
+std::vector Ranges(1, tooling::Range(0, Code.size()));
+FormattingAttemptStatus Status;
+tooling::Replacements Replaces =
+reformat(Style, Code, Ranges, "", );
+if (CheckComplete != SC_DoNotCheck) {
+  bool ExpectedCompleteFormat = CheckComplete == SC_ExpectComplete;
+  EXPECT_EQ(ExpectedCompleteFormat, Status.FormatComplete)
+  << Code << "\n\n";
+}
+auto Result = applyAllReplacements(Code, Replaces);
+EXPECT_TRUE(static_cast(Result));
+LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+return *Result;
+  }
+
+  FormatStyle getStyleWithColumns(FormatStyle Style, unsigned ColumnLimit) {
+Style.ColumnLimit = ColumnLimit;
+return Style;
+  }
+
+  FormatStyle getLLVMStyleWithColumns(unsigned ColumnLimit) {
+return getStyleWithColumns(getLLVMStyle(), ColumnLimit);
+  }
+
+  void _verifyFormat(const char *File, int Line, llvm::StringRef Expected,
+ llvm::StringRef Code,
+ const FormatStyle  = getLLVMStyle()) {
+ScopedTrace t(File, Line, ::testing::Message() << Code.str());
+EXPECT_EQ(Expected.str(), format(Expected, Style))
+<< "Expected code is not stable";
+EXPECT_EQ(Expected.str(), format(Code, Style));
+  }
+
+  void _verifyFormat(const char *File, int Line, llvm::StringRef 

[PATCH] D113393: [c++2b] Implement P0849R8 auto(x)

[Zhihao Yuan via Phabricator via cfe-commits]

lichray updated this revision to Diff 387016.
lichray marked 3 inline comments as done.
lichray added a comment.

Address review comments

- more test cases
- render `auto({})` and `new auto({})` invalid
- disambiguate `auto(x)->n` that looks like a declaration


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113393

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p5.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.auto.deduct/p2.cpp
  clang/test/CXX/expr/expr.post/expr.type.conv/p1-2b.cpp
  clang/test/CXX/expr/expr.unary/expr.new/p2-cxx0x.cpp
  clang/test/CXX/expr/expr.unary/expr.new/p2-cxx14.cpp
  clang/test/CXX/expr/expr.unary/expr.new/p2-cxx1z.cpp
  clang/test/Parser/cxx2b-auto-x.cpp
  clang/test/SemaCXX/deduced-return-type-cxx14.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1379,6 +1379,11 @@
   https://wg21.link/P2360R0;>P2360R0
   Clang 14
 
+
+  auto(x): decay-copy in the language
+  https://wg21.link/P0849R8;>P0849R8
+  Clang 14
+
 
 
 
Index: clang/test/SemaCXX/deduced-return-type-cxx14.cpp
===
--- clang/test/SemaCXX/deduced-return-type-cxx14.cpp
+++ clang/test/SemaCXX/deduced-return-type-cxx14.cpp
@@ -638,5 +638,7 @@
   using B = auto (*)() -> auto; // expected-error {{'auto' not allowed in type alias}}
   template auto> struct X {}; // cxx14-error {{'auto' not allowed in template parameter until C++17}}
   template struct Y { T x; };
-  Y auto> y; // expected-error {{'auto' not allowed in template argument}}
+  Y auto> y; // cxx14_20-error {{'auto' not allowed in template argument}} \
+  cxx2b-error {{initializer for functional-style cast to 'auto' is empty}} \
+  cxx2b-error {{expected unqualified-id}}
 }
Index: clang/test/Parser/cxx2b-auto-x.cpp
===
--- /dev/null
+++ clang/test/Parser/cxx2b-auto-x.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx2b -std=c++2b %s
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx20 -std=c++20 %s
+
+void looks_like_decltype_auto() {
+  decltype(auto(42)) b = 42; // cxx20-error {{'auto' not allowed here}}
+  decltype(long *) a = 42;   // expected-error {{expected '(' for function-style cast or type construction}} \
+expected-error {{expected expression}}
+  decltype(auto *) a = 42;   // expected-error {{expected '(' for function-style cast or type construction}} \
+expected-error {{expected expression}}
+  decltype(auto()) c = 42;   // cxx2b-error {{initializer for functional-style cast to 'auto' is empty}} \
+cxx20-error {{'auto' not allowed here}}
+}
+
+struct looks_like_declaration {
+  int n;
+} a;
+
+using T = looks_like_declaration *;
+void f() { T()->n = 1; }
+void g() { auto()->n = 0; } // cxx20-error {{declaration of variable 'a' with deduced type 'auto (&)' requires an initializer}} \
+ cxx20-error {{expected ';' at end of declaration}}
+void h() { auto{}->n = 0; } // cxx20-error {{expected unqualified-id}} \
+ cxx20-error {{expected expression}}
Index: clang/test/CXX/expr/expr.unary/expr.new/p2-cxx1z.cpp
===
--- clang/test/CXX/expr/expr.unary/expr.new/p2-cxx1z.cpp
+++ clang/test/CXX/expr/expr.unary/expr.new/p2-cxx1z.cpp
@@ -1,11 +1,30 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++14
 // RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++17 -pedantic
 
+// [expr.new]p2 ... the invented declaration: T x init ;
+// C++2b [dcl.type.auto.deduct]p2.2
+// For a variable declared with a type that contains a placeholder type, T is the declared type of the variable.
 void f() {
+  // - If the initializer is a parenthesized expression-list, the expression-list shall be a single assignmentexpression and E is the assignment-expression.
   new auto('a');
+  new decltype(auto)('a');
+  // - If the initializer is a braced-init-list, it shall consist of a single brace-enclosed assignment-expression and E is the assignment-expression.
   new auto {2};
+  new decltype(auto){2};
+
+  new auto {};   // expected-error{{new expression for type 'auto' requires a constructor argument}}
+  new auto({});  // expected-error{{cannot deduce actual type for 'auto' from parenthesized 

[PATCH] D113393: [c++2b] Implement P0849R8 auto(x)

[Zhihao Yuan via Phabricator via cfe-commits]

lichray marked 10 inline comments as done.
lichray added inline comments.



Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1032
+  // the typename-specifier in a function-style cast expression may
+  // be 'auto' since C++2b
   Diag(Tok.getLocation(),

rsmith wrote:
> Quuxplusone wrote:
> > rsmith wrote:
> > > Nice catch :)
> > I see zero hits for `git grep 'decltype[(]auto[^)] clang/`. So it seems 
> > this corner of the grammar has been missing any tests for a long time, but 
> > I hope this PR will add some.
> > ```
> > decltype(auto*) i = 42; // should be a syntax error
> > decltype(auto(42)) i = 42;  // should be OK 
> > decltype(auto()) i = 42;  // should be a syntax error
> > ```
> > Right now I don't see any tests for this stuff in this PR either. So it 
> > needs some.
> Some of this is tested in the new file 
> test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.auto.deduct/p2.cpp (you might 
> need to expand it manually; full-page search for `decltype(auto(` might 
> otherwise not find it). It looks like we are missing tests for things like 
> `decltype(auto*)`. It's not obvious to me what diagnostic would be most 
> useful if `decltype(auto` is not followed by `)`, `(`, or `{`, but my guess 
> is that "expected `)`" pointing at the `*`, rather than an error on the 
> `auto` token, would be the least surprising. So maybe this should be
> ```
> if (Tok.is(tok::kw_auto) && NextToken().isNot(tok::l_paren, tok::l_brace)) {
> ```
> ?
The r0 patch makes `decltype(auto *)` diagnose in the same way `decltype(long 
*)` do; looks good enough to me.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:1482-1486
+if (Exprs.size() == 1) {
+  if (auto p = dyn_cast_or_null(Exprs[0])) {
+Inits = MultiExprArg(p->getInits(), p->getNumInits());
+  }
+}

rsmith wrote:
> You should only do this if `ListInitialization` is `true`. Otherwise we'll 
> accept the invalid syntax `auto({a})`.
My bad. Deemed `new auto({a})` as valid; now corrected.



Comment at: 
clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.auto.deduct/p2.cpp:51
+f(A(*this));// ok
+f(auto(*this)); // ok in P0849
+  }

Quuxplusone wrote:
> Should you check that `__is_same(decltype(auto(*this)), A)`?
> Should you check that `__is_same(decltype(auto(this)), A*)`?
> Should you test inside a const member function too?
> Should you check that the friend function actually can call 
> `auto(some_a_object)`, the same way it can call `A(some_a_object)` — and more 
> to the point, that a non-friend //can't//?
All good ideas.




Comment at: clang/test/CXX/expr/expr.post/expr.type.conv/p1-2b.cpp:17-18
+
+  foo(auto(a));
+  foo(auto {a});
+  foo(auto(a));

Quuxplusone wrote:
> What is the significance of the whitespace here? Is this a lexer test?
> Normally it'd be just `auto(a)`, `auto{a}` — just like `T(a)`, `T{a}`. I 
> think we should follow convention (again, unless this is a lexer test — in 
> which case we should probably test //both// syntaxes with and without 
> whitespace).
Blame clang-format :/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113393

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

[PATCH] D113795: Comment Sema: Eliminate or factor out DeclInfo inspection (NFC)

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added inline comments.



Comment at: clang/include/clang/AST/CommentSema.h:205
 
-  /// \returns \c true if the declaration that this comment is attached to
-  /// is a pointer to function/method/block type or has such a type.
-  bool involvesFunctionType();
-
-  bool isFunctionDecl();
-  bool isAnyFunctionDecl();
-
-  /// \returns \c true if declaration that this comment is attached to declares
-  /// a function pointer.
-  bool isFunctionPointerVarDecl();
-  bool isFunctionOrMethodVariadic();
-  bool isObjCMethodDecl();
-  bool isObjCPropertyDecl();
-  bool isTemplateOrSpecialization();
-  bool isRecordLikeDecl();
-  bool isClassOrStructDecl();
-  /// \return \c true if the declaration that this comment is attached to
-  /// declares either struct, class or tag typedef.
-  bool isClassOrStructOrTagTypedefDecl();
-  bool isUnionDecl();
-  bool isObjCInterfaceDecl();
-  bool isObjCProtocolDecl();
-  bool isClassTemplateDecl();
-  bool isFunctionTemplateDecl();
+  bool checkDecl(bool (*check)(const DeclInfo &));
 

Another idea for naming: `hasDeclThat`, then calls read like 
`hasDeclThat(isOfSomeKind)`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113795

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

[PATCH] D113359: [Libomptarget][WIP] Introduce VGPU Plugin

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

I can't see it in the diff - does the cmake somewhere enable the existing tests 
on this new target?

A bit surprised to see ffi involved, are we thinking of spawning a separate 
process for the target?




Comment at: clang/lib/Basic/Targets/X86.h:49
 
+static const unsigned X86VGPUAddrSpaceMap[] = {
+0,   // Default

It's not clear to me what this is x86 specific. Being able to run our tests on 
power / arm etc seems like an advantage. Would also mean we would avoid adding 
openmp stuff the x86 specific files. Maybe OpenMPVGPUAddrSpaceMap and put it in 
one of the openmp source files?



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3988
+(T.isNVPTX() || T.isAMDGCN() ||
+ T.getVendor() == llvm::Triple::OpenMP_VGPU) &&
 Args.hasArg(options::OPT_fopenmp_cuda_mode);

Add a isOpenmpVGPU function?



Comment at: openmp/libomptarget/DeviceRTL/CMakeLists.txt:135
  -I${devicertl_base_directory}/../include
+ -I${devicertl_base_directory}/../plugins/vgpu/src
  ${LIBOMPTARGET_LLVM_INCLUDE_DIRS_DEVICERTL}

Should only add this include to the vgu, not all the plugins. May be able to 
use relative include paths to drop it entirely


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113359

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