[PATCH] D92753: [clang-format] Add IndentPragma style to eliminate common clang-format off scenario

[Marek Kurdej via Phabricator via cfe-commits]

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

LGTM with a single comment on the test.




Comment at: clang/unittests/Format/FormatTest.cpp:17676-17684
+format("void foo() {\n"
+   "#if 1\n"
+   "  #pragma omp simd\n"
+   "  for (int k = 0; k < 10; k++) {\n"
+   "#pragma omp simd\n"
+   "for (int j = 0; j < 10; j++) {\n"
+   "}\n"

Last nit, I'd mess up spaces like suggested here.


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

https://reviews.llvm.org/D92753

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

[PATCH] D90232: [clang-format] Formatting constructor initializer lists by putting them always on different lines (update to D14484)

[Marek Kurdej via Phabricator via cfe-commits]

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

I agree with @MyDeveloperDay that the existing tests should not be altered.




Comment at: clang/docs/ClangFormatStyleOptions.rst:1473
 
-**ConstructorInitializerAllOnOneLineOrOnePerLine** (``bool``)
-  If the constructor initializers don't fit on a line, put each
-  initializer on its own line.
+**ConstructorInitializer** (``ConstructorInitializerKind``)
+  Formatting of the constructor initializer.

MyDeveloperDay wrote:
> We can't easily remove an option once its been released, I know you have code 
> to handle it, but I think it needs to remain in the documentation and hence 
> the Format.h and somehow be marked deprecated otherwise someone is going to 
> look at the documentation and say, I could of sworn I used to be able to use 
> this setting...
I agree that `ConstructorInitializerAllOnOneLineOrOnePerLine` should be kept 
for backwards compatibility and translated to the new option 
`ConstructorInitializer`: `true` -> `BinPack` (`BestFit`) and `false` -> 
`Compact`, as it is done for example with 
`AlwaysBreakAfterDefinitionReturnType`.



Comment at: clang/docs/ClangFormatStyleOptions.rst:1481-1483
+  * ``CI_BestFit`` (in configuration: ``BestFit``)
+Eiter all initializers are on a single line (it this fits),
+otherwise every initializer has its own line.

Shouldn't that be called something like `BinPack` to match existing options 
(`BinPackArguments`, `BinPackParameters`)?



Comment at: clang/docs/ClangFormatStyleOptions.rst:1489-1500
 true:
 SomeClass::Constructor()
 : (), (), 
(a) {
   return 0;
 }
 
 false:

Please modify examples.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90232

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

[PATCH] D92715: [Clang][RISCV] Define RISC-V V builtin types

[Hsiangkai Wang via Phabricator via cfe-commits]

HsiangKai added inline comments.



Comment at: clang/include/clang/Basic/RISCVVTypes.def:67
+RVV_VECTOR_TYPE_INT("__rvv_int8m2_t",  RvvInt8m2,  RvvInt8m2Ty,  16,  8, 1, 
true)
+RVV_VECTOR_TYPE_INT("__rvv_int8m4_t",  RvvInt8m4,  RvvInt8m4Ty,  32,  8, 1, 
true)
+RVV_VECTOR_TYPE_INT("__rvv_int8m8_t",  RvvInt8m8,  RvvInt8m8Ty,  64,  8, 1, 
true)

kito-cheng wrote:
> khchen wrote:
> > evandro wrote:
> > > craig.topper wrote:
> > > > craig.topper wrote:
> > > > > jrtc27 wrote:
> > > > > > liaolucy wrote:
> > > > > > > RISC-V V has too many types, more than 200. All types use builtin 
> > > > > > > types? Is it possible to reduce the number of builtin types?
> > > > > > Indeed this is madness, what's wrong with just using 
> > > > > > `__attribute__((vector_size(n)))` on the right type? We should not 
> > > > > > be encouraging people to write code with architecture-specific 
> > > > > > types... but if we _really_ need these because RISC-V GCC decided 
> > > > > > this is how RISC-V V is going to look them can we not just shove 
> > > > > > them all in a header as typedef's for the architecture-independent 
> > > > > > attributed types and push that complexity out of the compiler 
> > > > > > itself?
> > > > > We are using  to specify types in IR. The size of 
> > > > > the fixed part is being used to control the LMUL parameter. There is 
> > > > > currently no way to spell a scalable vector type in C in a generic 
> > > > > way.
> > > > > 
> > > > > Alternatively I guess we could make LMUL a parameter to the intrinsic 
> > > > > and create the scalable IR types in the frontend based on it?
> > > > I do wonder why we bothered to have signed and unsigned types. The 
> > > > signedness of the operation should be carried in the intrinsic name.
> > > Some integer operations distinguish between signed and unsigned.  
> > > I do wonder why we bothered to have signed and unsigned types. The 
> > > signedness of the operation should be carried in the intrinsic name.
> > 
> > I think the only good thing for supporting both signed and unsigned type is 
> > that are more readable and compiler can does type checking for programmer. 
> > 
> > maybe the alternative way is changing intrinsic naming rule like 
> > 
> > ```
> > vint32m1_t a, b, c;
> > a = vadd_i32m1(b, c);
> > vuint32m1_t a, b, c;
> > a = vadd_u32m1(b, c);
> > vfloat32m1_t a, b, c;
> > a = vadd_f32m1(b, c);
> > ```
> One quick thought about this, if the concern is too much built-in types are 
> introduced in clang, maybe we could add a new attribute like 
> `__attribute__((vector_size(n)))`, maybe named 
> `__attribute__((riscv_scaleble_vector("[1|2|4|8|f2|f4|f8]")))`? and use that 
> to define vector types like `typedef int 
> __attribute__((riscv_scaleble_vector("2"))) vintm2_t`.
To have signed and unsigned types, we could do type checking for the builtins. 
For example, we have vmaxu_vv and vmax_vv. Users could pass signed vector types 
into unsigned version vmaxu_vv if we only have types without signed/unsigned. 
How could we tell users they pass values into the wrong builtins?


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

https://reviews.llvm.org/D92715

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

[PATCH] D92715: [Clang][RISCV] Define RISC-V V builtin types

[Liao Chunyu via Phabricator via cfe-commits]

liaolucy added inline comments.



Comment at: clang/include/clang/Basic/RISCVVTypes.def:68
+RVV_VECTOR_TYPE_INT("__rvv_int8mf2_t", RvvInt8mf2, RvvInt8mf2Ty, 4,  8, 1, 
true)
+RVV_VECTOR_TYPE_INT("__rvv_int8m1_t",  RvvInt8m1,  RvvInt8m1Ty,  8,  8, 1, 
true)
+RVV_VECTOR_TYPE_INT("__rvv_int8m2_t",  RvvInt8m2,  RvvInt8m2Ty,  16, 8, 1, 
true)

eg: typedef __attribute__((riscv_vector_type(16/*EIBits*/, 1/*LMUL*/, ...)))  
int16_t vint16m1_t;
Please help to consider if this is feasible.


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

https://reviews.llvm.org/D92715

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

[PATCH] D92753: [clang-format] Add IndentPragma style to eliminate common clang-format off scenario

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:17676-17684
+format("void foo() {\n"
+   "#if 1\n"
+   "  #pragma omp simd\n"
+   "  for (int k = 0; k < 10; k++) {\n"
+   "#pragma omp simd\n"
+   "for (int j = 0; j < 10; j++) {\n"
+   "}\n"

curdeius wrote:
> Last nit, I'd mess up spaces like suggested here.
This is `BeforeHash` not `AfterHash`, I think the test is correct (as it passes)


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

https://reviews.llvm.org/D92753

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

[PATCH] D92715: [Clang][RISCV] Define RISC-V V builtin types

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/include/clang/Basic/RISCVVTypes.def:67
+RVV_VECTOR_TYPE_INT("__rvv_int8m2_t",  RvvInt8m2,  RvvInt8m2Ty,  16,  8, 1, 
true)
+RVV_VECTOR_TYPE_INT("__rvv_int8m4_t",  RvvInt8m4,  RvvInt8m4Ty,  32,  8, 1, 
true)
+RVV_VECTOR_TYPE_INT("__rvv_int8m8_t",  RvvInt8m8,  RvvInt8m8Ty,  64,  8, 1, 
true)

HsiangKai wrote:
> kito-cheng wrote:
> > khchen wrote:
> > > evandro wrote:
> > > > craig.topper wrote:
> > > > > craig.topper wrote:
> > > > > > jrtc27 wrote:
> > > > > > > liaolucy wrote:
> > > > > > > > RISC-V V has too many types, more than 200. All types use 
> > > > > > > > builtin types? Is it possible to reduce the number of builtin 
> > > > > > > > types?
> > > > > > > Indeed this is madness, what's wrong with just using 
> > > > > > > `__attribute__((vector_size(n)))` on the right type? We should 
> > > > > > > not be encouraging people to write code with 
> > > > > > > architecture-specific types... but if we _really_ need these 
> > > > > > > because RISC-V GCC decided this is how RISC-V V is going to look 
> > > > > > > them can we not just shove them all in a header as typedef's for 
> > > > > > > the architecture-independent attributed types and push that 
> > > > > > > complexity out of the compiler itself?
> > > > > > We are using  to specify types in IR. The size of 
> > > > > > the fixed part is being used to control the LMUL parameter. There 
> > > > > > is currently no way to spell a scalable vector type in C in a 
> > > > > > generic way.
> > > > > > 
> > > > > > Alternatively I guess we could make LMUL a parameter to the 
> > > > > > intrinsic and create the scalable IR types in the frontend based on 
> > > > > > it?
> > > > > I do wonder why we bothered to have signed and unsigned types. The 
> > > > > signedness of the operation should be carried in the intrinsic name.
> > > > Some integer operations distinguish between signed and unsigned.  
> > > > I do wonder why we bothered to have signed and unsigned types. The 
> > > > signedness of the operation should be carried in the intrinsic name.
> > > 
> > > I think the only good thing for supporting both signed and unsigned type 
> > > is that are more readable and compiler can does type checking for 
> > > programmer. 
> > > 
> > > maybe the alternative way is changing intrinsic naming rule like 
> > > 
> > > ```
> > > vint32m1_t a, b, c;
> > > a = vadd_i32m1(b, c);
> > > vuint32m1_t a, b, c;
> > > a = vadd_u32m1(b, c);
> > > vfloat32m1_t a, b, c;
> > > a = vadd_f32m1(b, c);
> > > ```
> > One quick thought about this, if the concern is too much built-in types are 
> > introduced in clang, maybe we could add a new attribute like 
> > `__attribute__((vector_size(n)))`, maybe named 
> > `__attribute__((riscv_scaleble_vector("[1|2|4|8|f2|f4|f8]")))`? and use 
> > that to define vector types like `typedef int 
> > __attribute__((riscv_scaleble_vector("2"))) vintm2_t`.
> To have signed and unsigned types, we could do type checking for the 
> builtins. For example, we have vmaxu_vv and vmax_vv. Users could pass signed 
> vector types into unsigned version vmaxu_vv if we only have types without 
> signed/unsigned. How could we tell users they pass values into the wrong 
> builtins?
Maybe I've been corrupted by X86 where there is just a single 128-bit integer 
vector type. Even though there are 8, 16, 32, 64 bit element operations with 
some being signed/unsigned.


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

https://reviews.llvm.org/D92715

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

[PATCH] D85788: [Clang test] Update to allow passing extra default clang arguments in use_clang

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added a comment.
Herald added a subscriber: frasercrmck.

In D85788#2335838 , @eugenis wrote:

> I wonder it can be avoided by
>
> - disable noundef analysis by default in cc1
> - always add -enable-noundef-analysis in the driver when invoking cc1

Would this cause more tests that use `%clang` to be updated, such as  
`test/CodeGen/mips-vector-return.c`?
IIUC, the tests in this patch are okay to have their `%clang` replaced with 
`%clang_bin` because they are not checking function signatures.

> I don't like the %clang_bin substitution - imho it's unclear for the test 
> authors when to use it instead of %clang, but I can't come up with a better 
> idea.

From a user's perspective, simply naming it as `%clang_noundef` would be 
clearer, IMO.
This new substitution is going to be used for the noundef checks in D81678 
 only anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85788

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

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added a comment.

In D87974#2438723 , @zoecarver wrote:

> In D87974#2438682 , @BillyONeal 
> wrote:
>
>>> Are they actually the same, with the same handling of corner cases like 
>>> unions and tail padding?
>>> There's more to this than just the name, and if they aren't the same, it 
>>> seems better to have two names.
>>
>> They are both implementing the same C++ feature, with the same desired 
>> semantics of zeroing out any bits in the object representation that aren't 
>> in the value representation. If they differ, one or the other would have a 
>> bug.

Do they support non-trivially copyable types? That isn't required for the 
atomic compare exchange feature, but is relevant for a feature exposed to 
users. What about extensions like zero-sized arrays or C99 flexible array 
members?

> I agree, they either need to be identical (including corner cases) or there 
> needs to be two of them (i.e., GCC ships both `__builtin_zero_non_value_bits` 
> and `__builtin_clear_padding` and the first is the same as MSVC, Clang, and 
> NVCC).

GCC doesn't need to support both. It only works with libstdc++ so it only needs 
to support the one used by libstdc++ (although there is a patch to add 
`-stdlib=libc++` to GCC).

If libstdc++ uses `__has_builtin` to check what the compiler supports then 
Clang doesn't even need to support GCC's built-in, because libstdc++ wouldn't 
use it if not supported (and could use `__builtin_zero_non_value_bits` instead 
when supported).

The Intel compiler would need to support both though.

>>> Is there a specification for __builtin_zero_non_value_bits available 
>>> somewhere?
>>
>> I don't know if there is a formal spec for it beyond the actual C++ standard.
>
> I think P0528 is the relevant paper but other than that, no, there's not a 
> spec. I think that's going to be the most time sensitive part of implementing 
> this: coming up with the spec and making sure all the tests pass on all the 
> implementations.

GCC has publicly available documentation describing its built-in, and publicly 
available tests for it. That's the kind of spec I'm looking for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87974

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

[clang] a134477 - Revert "Add new 'preferred_name' attribute."

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-12-08T00:42:48-08:00
New Revision: a1344779ab019a6bcd29842c1499343e15efbe87

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

LOG: Revert "Add new 'preferred_name' attribute."

This change exposed a pre-existing issue with deserialization cycles
caused by a combination of attributes and template instantiations
violating the deserialization ordering restrictions; see PR48434 for
details.

A previous commit attempted to work around PR48434, but appears to have
only been a partial fix, and fixing this properly seems non-trivial.
Backing out for now to unblock things.

This reverts commit 98f76adf4e941738c0b9fe3b9965fa63603e9c89 and
commit a64c26a47a81b1b44e36d235ff3bc6a74a0bad9f.

Added: 


Modified: 
clang/include/clang/AST/TypeProperties.td
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/AST/TypePrinter.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/lib/Sema/SemaTemplate.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/test/SemaTemplate/attributes.cpp
clang/utils/TableGen/ClangAttrEmitter.cpp
libcxx/include/__config
libcxx/include/iosfwd
libcxx/include/regex
libcxx/include/string
libcxx/include/string_view

Removed: 
clang/test/PCH/decl-attrs.cpp



diff  --git a/clang/include/clang/AST/TypeProperties.td 
b/clang/include/clang/AST/TypeProperties.td
index b582395c44a6..a183ac0479c6 100644
--- a/clang/include/clang/AST/TypeProperties.td
+++ b/clang/include/clang/AST/TypeProperties.td
@@ -484,12 +484,8 @@ let Class = TagType in {
 let Read = [{ node->isDependentType() }];
   }
   def : Property<"declaration", DeclRef> {
-// We don't know which declaration was originally referenced here, and we
-// cannot reference a declaration that follows the use (because that can
-// introduce deserialization cycles), so conservatively generate a
-// reference to the first declaration.
-// FIXME: If this is a reference to a class template specialization, that
-// can still introduce a deserialization cycle.
+// Serializing a reference to the canonical declaration is apparently
+// necessary to make module-merging work.
 let Read = [{ node->getDecl()->getCanonicalDecl() }];
   }
 }

diff  --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index 51f654fc7613..52041201d22f 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -126,9 +126,6 @@ def FunctionTmpl
  FunctionDecl::TK_FunctionTemplate}],
 "function templates">;
 
-def ClassTmpl : SubsetSubjectgetDescribedClassTemplate()}],
-  "class templates">;
-
 // FIXME: this hack is needed because DeclNodes.td defines the base Decl node
 // type to be a class, not a definition. This makes it impossible to create an
 // attribute subject which accepts a Decl. Normally, this is not a problem,
@@ -2394,16 +2391,6 @@ def Pascal : DeclOrTypeAttr {
   let Documentation = [Undocumented];
 }
 
-def PreferredName : InheritableAttr {
-  let Spellings = [Clang<"preferred_name", /*AllowInC*/0>];
-  let Subjects = SubjectList<[ClassTmpl]>;
-  let Args = [TypeArgument<"TypedefType">];
-  let Documentation = [PreferredNameDocs];
-  let InheritEvenIfAlreadyPresent = 1;
-  let MeaningfulToClassTemplateDefinition = 1;
-  let TemplateDependent = 1;
-}
-
 def PreserveMost : DeclOrTypeAttr {
   let Spellings = [Clang<"preserve_most">];
   let Documentation = [PreserveMostDocs];

diff  --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index 28f35cf2c0c7..e7e2805a9608 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -4471,30 +4471,6 @@ the old mangled name and the new code will use the new 
mangled name with tags.
   }];
 }
 
-def PreferredNameDocs : Documentation {
-  let Category = DocCatDecl;
-  let Content = [{
-The ``preferred_name`` attribute can be applied to a class template, and
-specifies a preferred way of naming a specialization of the template. The
-preferred name will be used whenever the corresponding template specialization
-would otherwise be printed in a diagnostic or similar context.
-
-The preferred name must be a typedef or type alias declaration that refers to a
-specialization of the class template (not including any type qualifiers). In
-general this requires the template to be declared at least twice. For example:
-
-.. code-block:: c++
-
-  template struct basic_string;
-  using string = basic_string;
-  using wstring = basic_string;
-  template struct [[clang::preferred_name(string),
- 

[PATCH] D92822: [clang-format] [NFC] Fix spelling and grammatical errors in IncludeBlocks text

[MyDeveloperDay via Phabricator via cfe-commits]

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

Fix spelling mistake
Leave space after `.` and before beginning of next sentence
Reword it slightly to try and make it more readable.
Ensure RST is updated correctly (it generated a change)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92822

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Tooling/Inclusions/IncludeStyle.h


Index: clang/include/clang/Tooling/Inclusions/IncludeStyle.h
===
--- clang/include/clang/Tooling/Inclusions/IncludeStyle.h
+++ clang/include/clang/Tooling/Inclusions/IncludeStyle.h
@@ -89,12 +89,11 @@
   /// always need to be first.
   ///
   /// There is a third and optional field ``SortPriority`` which can used while
-  /// ``IncludeBloks = IBS_Regroup`` to define the priority in which
-  /// ``#includes`` should be ordered, and value of ``Priority`` defines the
-  /// order of
-  /// ``#include blocks`` and also enables to group ``#includes`` of different
-  /// priority for order.``SortPriority`` is set to the value of ``Priority``
-  /// as default if it is not assigned.
+  /// ``IncludeBlocks = IBS_Regroup`` to define the priority in which
+  /// ``#includes`` should be ordered. The value of ``Priority`` defines the
+  /// order of ``#include blocks`` and also allows the grouping of 
``#includes``
+  /// of different priority. ``SortPriority`` is set to the value of
+  /// ``Priority`` as default if it is not assigned.
   ///
   /// Each regular expression can be marked as case sensitive with the field
   /// ``CaseSensitive``, per default it is not.
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1713,11 +1713,11 @@
   always need to be first.
 
   There is a third and optional field ``SortPriority`` which can used while
-  ``IncludeBloks = IBS_Regroup`` to define the priority in which ``#includes``
-  should be ordered, and value of ``Priority`` defines the order of
-  ``#include blocks`` and also enables to group ``#includes`` of different
-  priority for order.``SortPriority`` is set to the value of ``Priority``
-  as default if it is not assigned.
+  ``IncludeBlocks = IBS_Regroup`` to define the priority in which
+  ``#includes`` should be ordered. The value of ``Priority`` defines the
+  order of ``#include blocks`` and also allows the grouping of ``#includes``
+  of different priority. ``SortPriority`` is set to the value of
+  ``Priority`` as default if it is not assigned.
 
   Each regular expression can be marked as case sensitive with the field
   ``CaseSensitive``, per default it is not.


Index: clang/include/clang/Tooling/Inclusions/IncludeStyle.h
===
--- clang/include/clang/Tooling/Inclusions/IncludeStyle.h
+++ clang/include/clang/Tooling/Inclusions/IncludeStyle.h
@@ -89,12 +89,11 @@
   /// always need to be first.
   ///
   /// There is a third and optional field ``SortPriority`` which can used while
-  /// ``IncludeBloks = IBS_Regroup`` to define the priority in which
-  /// ``#includes`` should be ordered, and value of ``Priority`` defines the
-  /// order of
-  /// ``#include blocks`` and also enables to group ``#includes`` of different
-  /// priority for order.``SortPriority`` is set to the value of ``Priority``
-  /// as default if it is not assigned.
+  /// ``IncludeBlocks = IBS_Regroup`` to define the priority in which
+  /// ``#includes`` should be ordered. The value of ``Priority`` defines the
+  /// order of ``#include blocks`` and also allows the grouping of ``#includes``
+  /// of different priority. ``SortPriority`` is set to the value of
+  /// ``Priority`` as default if it is not assigned.
   ///
   /// Each regular expression can be marked as case sensitive with the field
   /// ``CaseSensitive``, per default it is not.
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1713,11 +1713,11 @@
   always need to be first.
 
   There is a third and optional field ``SortPriority`` which can used while
-  ``IncludeBloks = IBS_Regroup`` to define the priority in which ``#includes``
-  should be ordered, and value of ``Priority`` defines the order of
-  ``#include blocks`` and also enables to group ``#includes`` of different
-  priority for order.``SortPriority`` is set to the value of ``Priority``
-  as default if it is not assigned.
+  ``IncludeBlocks = IBS_Regroup`` to define the priority in which
+  ``#includes`` should be ordered. The value of ``Priority`` d

[PATCH] D92822: [clang-format] [NFC] Fix spelling and grammatical errors in IncludeBlocks text

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

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

I have only one question, what stands NFC for? I figured something like "no 
functional change"? But I can't find a definition anywhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92822

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

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

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

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

In D92257#2438926 , @MyDeveloperDay 
wrote:

> In D92257#2435906 , @lebedev.ri 
> wrote:
>
>> In D92257#2435902 , @MyDeveloperDay 
>> wrote:
>>
>>> In D92257#2435899 , 
>>> @HazardyKnusperkeks wrote:
>>>
 In D92257#2435701 , 
 @MyDeveloperDay wrote:

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

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

 You can set me as author:
 `Björn Schäpers `
 My Github Account is also called `HazardyKnusperkeks`.
>>>
>>> The process is that you add (https://llvm.org/docs/Contributing.html)
>>>
>>> Patch By: HazardyKnusperkeks
>>>
>>> to the commit message if the user doesn't have commit access, if you want 
>>> your name against the blame then I recommend applying for commit access 
>>> yourself.
>>
>> That is incorrect and does not represent the nowadays reality, i suggest 
>> that you look up the docs.
>>
>>> let me know if you still want me to land this
>
> Yes I agree I hadn’t seen that the process had changed,
>
> This is one reason why I don’t like landing patches for others, this just 
> confirms that in the future I will generally request people apply for commit 
> access themselves.

And where do I do that? Also I did not think I would not have a chance of 
getting the access so early.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92257

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

[clang] ca93f9a - [Clang][CodeGen][RISCV] Add hard float ABI tests with empty struct

[Luís Marques via cfe-commits]

Author: Luís Marques
Date: 2020-12-08T09:19:05Z
New Revision: ca93f9abdc0abc96ca8fb7999549a50aadd95caf

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

LOG: [Clang][CodeGen][RISCV] Add hard float ABI tests with empty struct

This patch adds tests that showcase a behavior that is currently buggy.
Fix in a follow-up patch.

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

Added: 
clang/test/CodeGen/riscv32-ilp32d-abi.cpp

Modified: 


Removed: 




diff  --git a/clang/test/CodeGen/riscv32-ilp32d-abi.cpp 
b/clang/test/CodeGen/riscv32-ilp32d-abi.cpp
new file mode 100644
index ..ffebb057e230
--- /dev/null
+++ b/clang/test/CodeGen/riscv32-ilp32d-abi.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -triple riscv32 -target-feature +d -target-abi ilp32d \
+// RUN: -Wno-missing-declarations -emit-llvm %s -o - | FileCheck %s
+
+struct empty_float2 { struct {}; float f; float g; };
+
+// CHECK: define float @_Z14f_empty_float212empty_float2(float %0, float %1)
+// FIXME: Extraneous padding before the second float
+// CHECK: { [4 x i8], float, [4 x i8], float }
+float f_empty_float2(empty_float2 a) {
+return a.g;
+}
+
+struct empty_double2 { struct {}; double f; double g; };
+
+// CHECK: define double @_Z15f_empty_double213empty_double2(double %0, double 
%1)
+// FIXME: Extraneous padding before the second double
+// CHECK: { [8 x i8], double, [8 x i8], double }
+double f_empty_double2(empty_double2 a) {
+return a.g;
+}
+
+struct empty_float_double { struct {}; float f; double g; };
+
+// CHECK: define double @_Z20f_empty_float_double18empty_float_double(float 
%0, double %1)
+// CHECK: { [4 x i8], float, double }
+double f_empty_float_double(empty_float_double a) {
+return a.g;
+}
+
+struct empty_double_float { struct {}; double f; float g; };
+
+// CHECK: define double @_Z20f_empty_double_float18empty_double_float(double 
%0, float %1)
+// FIXME: Extraneous padding before the float
+// CHECK: { [8 x i8], double, [8 x i8], float }
+double f_empty_double_float(empty_double_float a) {
+return a.g;
+}



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

[clang] fa8f5bf - [Clang][CodeGen][RISCV] Fix hard float ABI test cases with empty struct

[Luís Marques via cfe-commits]

Author: Luís Marques
Date: 2020-12-08T09:19:05Z
New Revision: fa8f5bfa4e8cff042c9730320c74e97fab152ae1

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

LOG: [Clang][CodeGen][RISCV] Fix hard float ABI test cases with empty struct

The code seemed not to account for the field 1 offset.

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

Added: 


Modified: 
clang/lib/CodeGen/TargetInfo.cpp
clang/test/CodeGen/riscv32-ilp32d-abi.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/TargetInfo.cpp 
b/clang/lib/CodeGen/TargetInfo.cpp
index 7213f7864d43..05c12dfe0458 100644
--- a/clang/lib/CodeGen/TargetInfo.cpp
+++ b/clang/lib/CodeGen/TargetInfo.cpp
@@ -10520,7 +10520,7 @@ bool RISCVABIInfo::detectFPCCEligibleStruct(QualType 
Ty, llvm::Type *&Field1Ty,
 NeededArgFPRs++;
   else if (Field2Ty)
 NeededArgGPRs++;
-  return IsCandidate;
+  return true;
 }
 
 // Call getCoerceAndExpand for the two-element flattened struct described by
@@ -10546,15 +10546,15 @@ ABIArgInfo 
RISCVABIInfo::coerceAndExpandFPCCEligibleStruct(
 
   CharUnits Field2Align =
   CharUnits::fromQuantity(getDataLayout().getABITypeAlignment(Field2Ty));
-  CharUnits Field1Size =
+  CharUnits Field1End = Field1Off +
   CharUnits::fromQuantity(getDataLayout().getTypeStoreSize(Field1Ty));
-  CharUnits Field2OffNoPadNoPack = Field1Size.alignTo(Field2Align);
+  CharUnits Field2OffNoPadNoPack = Field1End.alignTo(Field2Align);
 
   CharUnits Padding = CharUnits::Zero();
   if (Field2Off > Field2OffNoPadNoPack)
 Padding = Field2Off - Field2OffNoPadNoPack;
-  else if (Field2Off != Field2Align && Field2Off > Field1Size)
-Padding = Field2Off - Field1Size;
+  else if (Field2Off != Field2Align && Field2Off > Field1End)
+Padding = Field2Off - Field1End;
 
   bool IsPacked = !Field2Off.isMultipleOf(Field2Align);
 

diff  --git a/clang/test/CodeGen/riscv32-ilp32d-abi.cpp 
b/clang/test/CodeGen/riscv32-ilp32d-abi.cpp
index ffebb057e230..1018c78e168b 100644
--- a/clang/test/CodeGen/riscv32-ilp32d-abi.cpp
+++ b/clang/test/CodeGen/riscv32-ilp32d-abi.cpp
@@ -4,8 +4,7 @@
 struct empty_float2 { struct {}; float f; float g; };
 
 // CHECK: define float @_Z14f_empty_float212empty_float2(float %0, float %1)
-// FIXME: Extraneous padding before the second float
-// CHECK: { [4 x i8], float, [4 x i8], float }
+// CHECK: { [4 x i8], float, float }
 float f_empty_float2(empty_float2 a) {
 return a.g;
 }
@@ -13,8 +12,7 @@ float f_empty_float2(empty_float2 a) {
 struct empty_double2 { struct {}; double f; double g; };
 
 // CHECK: define double @_Z15f_empty_double213empty_double2(double %0, double 
%1)
-// FIXME: Extraneous padding before the second double
-// CHECK: { [8 x i8], double, [8 x i8], double }
+// CHECK: { [8 x i8], double, double }
 double f_empty_double2(empty_double2 a) {
 return a.g;
 }
@@ -30,8 +28,7 @@ double f_empty_float_double(empty_float_double a) {
 struct empty_double_float { struct {}; double f; float g; };
 
 // CHECK: define double @_Z20f_empty_double_float18empty_double_float(double 
%0, float %1)
-// FIXME: Extraneous padding before the float
-// CHECK: { [8 x i8], double, [8 x i8], float }
+// CHECK: { [8 x i8], double, float }
 double f_empty_double_float(empty_double_float a) {
 return a.g;
 }



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

[clang] 3af354e - [Clang][CodeGen][RISCV] Fix hard float ABI for struct with empty struct and complex

[Luís Marques via cfe-commits]

Author: Luís Marques
Date: 2020-12-08T09:19:05Z
New Revision: 3af354e863f553ef727967dfc091a64a11500aa5

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

LOG: [Clang][CodeGen][RISCV] Fix hard float ABI for struct with empty struct 
and complex

Fixes bug 44904.

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

Added: 


Modified: 
clang/lib/CodeGen/TargetInfo.cpp
clang/test/CodeGen/riscv32-ilp32d-abi.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/TargetInfo.cpp 
b/clang/lib/CodeGen/TargetInfo.cpp
index 05c12dfe0458..d4191b943ef5 100644
--- a/clang/lib/CodeGen/TargetInfo.cpp
+++ b/clang/lib/CodeGen/TargetInfo.cpp
@@ -10425,7 +10425,6 @@ bool 
RISCVABIInfo::detectFPCCEligibleStructHelper(QualType Ty, CharUnits CurOff,
   return false;
 Field1Ty = CGT.ConvertType(EltTy);
 Field1Off = CurOff;
-assert(CurOff.isZero() && "Unexpected offset for first field");
 Field2Ty = Field1Ty;
 Field2Off = Field1Off + getContext().getTypeSizeInChars(EltTy);
 return true;

diff  --git a/clang/test/CodeGen/riscv32-ilp32d-abi.cpp 
b/clang/test/CodeGen/riscv32-ilp32d-abi.cpp
index 1018c78e168b..26d968be97df 100644
--- a/clang/test/CodeGen/riscv32-ilp32d-abi.cpp
+++ b/clang/test/CodeGen/riscv32-ilp32d-abi.cpp
@@ -32,3 +32,19 @@ struct empty_double_float { struct {}; double f; float g; };
 double f_empty_double_float(empty_double_float a) {
 return a.g;
 }
+
+struct empty_complex_f { struct {}; float _Complex fc; };
+
+// CHECK: define float @_Z17f_empty_complex_f15empty_complex_f(float %0, float 
%1)
+// CHECK: { [4 x i8], float, float }
+float f_empty_complex_f(empty_complex_f a) {
+return __imag__ a.fc;
+}
+
+struct empty_complex_d { struct {}; double _Complex fc; };
+
+// CHECK: define double @_Z17f_empty_complex_d15empty_complex_d(double %0, 
double %1)
+// CHECK: { [8 x i8], double, double }
+double f_empty_complex_d(empty_complex_d a) {
+return __imag__ a.fc;
+}



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

[PATCH] D92792: [clang] - Also look for devtoolset-10

[Stephan Dollberg via Phabricator via cfe-commits]

stephan.dollberg added a comment.

Thanks @phosek , can you merge it please? I don't have perms.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92792

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

[PATCH] D92822: [clang-format] [NFC] Fix spelling and grammatical errors in IncludeBlocks text

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.

In D92822#2439069 , 
@HazardyKnusperkeks wrote:

> I have only one question, what stands NFC for? I figured something like "no 
> functional change"? But I can't find a definition anywhere.

https://www.llvm.org/docs/Lexicon.html#nfc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92822

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

[PATCH] D92753: [clang-format] Add IndentPragma style to eliminate common clang-format off scenario

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:17676-17684
+format("void foo() {\n"
+   "#if 1\n"
+   "  #pragma omp simd\n"
+   "  for (int k = 0; k < 10; k++) {\n"
+   "#pragma omp simd\n"
+   "for (int j = 0; j < 10; j++) {\n"
+   "}\n"

MyDeveloperDay wrote:
> curdeius wrote:
> > Last nit, I'd mess up spaces like suggested here.
> This is `BeforeHash` not `AfterHash`, I think the test is correct (as it 
> passes)
Ok. My thought was that we were not really checking that this patch will 
correctly reformat pragmas that are not yet on correct columns, but after your 
change to remove messUp=false, we do test it.
Again, LGTM! And thank you for the patch (and the patience) :).


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

https://reviews.llvm.org/D92753

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

[PATCH] D92753: [clang-format] Add IndentPragma style to eliminate common clang-format off scenario

[MyDeveloperDay via Phabricator via cfe-commits]

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



Comment at: clang/unittests/Format/FormatTest.cpp:17676-17684
+format("void foo() {\n"
+   "#if 1\n"
+   "  #pragma omp simd\n"
+   "  for (int k = 0; k < 10; k++) {\n"
+   "#pragma omp simd\n"
+   "for (int j = 0; j < 10; j++) {\n"
+   "}\n"

curdeius wrote:
> MyDeveloperDay wrote:
> > curdeius wrote:
> > > Last nit, I'd mess up spaces like suggested here.
> > This is `BeforeHash` not `AfterHash`, I think the test is correct (as it 
> > passes)
> Ok. My thought was that we were not really checking that this patch will 
> correctly reformat pragmas that are not yet on correct columns, but after 
> your change to remove messUp=false, we do test it.
> Again, LGTM! And thank you for the patch (and the patience) :).
Its never a problem @curdeius, I'm always happy to wait for your feedback as 
its considered and insightful, your comment made be go back and double check 
and that can never be a bad thing. Thanks for the accept.


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

https://reviews.llvm.org/D92753

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

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

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.



> And where do I do that? Also I did not think I would not have a chance of 
> getting the access so early.

https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92257

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

[PATCH] D89959: UBSAN: emit distinctive traps in trapping mode

[Tim Northover via Phabricator via cfe-commits]

t.p.northover closed this revision.
t.p.northover added a comment.

Committed:

To github.com:llvm/llvm-project.git

  c54d827fdb12..c5978f42ec8e  main -> main


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

https://reviews.llvm.org/D89959

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

[PATCH] D92788: [clangd] NFC: Use SmallVector where possible

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks for cleaning up!

In D92788#2438207 , @njames93 wrote:

> Not sure I'm a huge fan of this, Some of these cases the size is specified 
> because that's the upper limit we typically expect the SmallVector to use.

The idea behind the new LLVM-wide recommendation is that in principle this is 
true but in practice it's mostly false (at least, these estimates often aren't 
chosen carefully).

This seems true to me - we do try to guess a sensible value (when forced to 
choose something), but we probably shouldn't be specifying these where we 
wouldn't choose to e.g. `reserve()` in a std::vector.
There's some readability/maintenance cost and in most cases no performance 
difference at the level we typically care about.

I've flagged cases where it seems like it might matter - please do the same if 
you find any!




Comment at: clang-tools-extra/clangd/CompileCommands.cpp:334
   using TableTy =
-  llvm::StringMap, llvm::BumpPtrAllocator>;
+  llvm::StringMap, llvm::BumpPtrAllocator>;
   static TableTy *Table = [] {

This is significant in size (around half a megabyte per the comment on 
ArgStripper). Rule is 24 bytes, so we're reducing N from 4 to 2 I believe.
Please leave this alone or check that it's a reduction in memory usage (if 
memory usage is roughly equal, fewer heap allocations is better).



Comment at: clang-tools-extra/clangd/Selection.cpp:784
 // Always returns at least one range - if no tokens touched, and empty range.
-static llvm::SmallVector, 2>
+static llvm::SmallVector>
 pointBounds(unsigned Offset, const syntax::TokenBuffer &Tokens) {

I'd prefer to keep the 2 here for readability, it's a nice hint/reminder that 
we *always* return 1 or 2 values



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:1414
   History = History.take_back(15);
-  llvm::SmallVector Recent(History.begin(), 
History.end());
+  llvm::SmallVector Recent(History.begin(), History.end());
   auto Median = Recent.begin() + Recent.size() / 2;

This *always* allocates and the previous version *never* allocates, please 
revert this one.



Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:330
   {
-llvm::DenseMap> MergedRefs;
+llvm::DenseMap> MergedRefs;
 size_t Count = 0;

this is a reduction from 4 to 2, and this is fairly performance-sensitive code 
(for the preamble index).
My intuition is that not that many symbols are referenced 1-2 times across all 
TUs, and when many are, it's a tiny project where performance doesn't matter.
Guessing about performance is probably folly. But I'd probably avoid reducing 
this without measuring.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92788

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

[PATCH] D92108: Fix inconsistent availability attribute message string literal check.

[Nigel Perks via Phabricator via cfe-commits]

nigelp-xmos added a comment.

Many thanks for review and approval. Please could it be committed as I do not 
have commit access? (I will request.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92108

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

[PATCH] D92764: [analyzer][StdLibraryFunctionsChecker] Make close and mmap to accept -1 as fd

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

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

Thank you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92764

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

[PATCH] D92825: [clang][cli] CompilerInvocationTest: join two test fixtures into one

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added a reviewer: dexonsmith.
Herald added a reviewer: a.sidorin.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92825

Files:
  clang/unittests/Frontend/CompilerInvocationTest.cpp

Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -22,29 +22,22 @@
 using ::testing::StrNe;
 
 namespace {
-struct OptsPopulationTest : public ::testing::Test {
-  IntrusiveRefCntPtr Diags;
-  CompilerInvocation CInvok;
-
-  OptsPopulationTest()
-  : Diags(CompilerInstance::createDiagnostics(new DiagnosticOptions())) {}
-};
-
-class CC1CommandLineGenerationTest : public ::testing::Test {
+class CommandLineTest : public ::testing::Test {
 public:
   IntrusiveRefCntPtr Diags;
   SmallVector GeneratedArgs;
   SmallVector GeneratedArgsStorage;
+  CompilerInvocation CInvok;
 
   const char *operator()(const Twine &Arg) {
 return GeneratedArgsStorage.emplace_back(Arg.str()).c_str();
   }
 
-  CC1CommandLineGenerationTest()
+  CommandLineTest()
   : Diags(CompilerInstance::createDiagnostics(new DiagnosticOptions())) {}
 };
 
-TEST_F(OptsPopulationTest, OptIsInitializedWithCustomDefaultValue) {
+TEST_F(CommandLineTest, OptIsInitializedWithCustomDefaultValue) {
   const char *Args[] = {"clang", "-xc++"};
 
   CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
@@ -52,7 +45,7 @@
   ASSERT_TRUE(CInvok.getFrontendOpts().UseTemporary);
 }
 
-TEST_F(OptsPopulationTest, OptOfNegativeFlagIsPopulatedWithFalse) {
+TEST_F(CommandLineTest, OptOfNegativeFlagIsPopulatedWithFalse) {
   const char *Args[] = {"clang", "-xc++", "-fno-temp-file"};
 
   CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
@@ -60,7 +53,7 @@
   ASSERT_FALSE(CInvok.getFrontendOpts().UseTemporary);
 }
 
-TEST_F(OptsPopulationTest, OptsOfImpliedPositiveFlagArePopulatedWithTrue) {
+TEST_F(CommandLineTest, OptsOfImpliedPositiveFlagArePopulatedWithTrue) {
   const char *Args[] = {"clang", "-xc++", "-cl-unsafe-math-optimizations"};
 
   CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
@@ -76,10 +69,9 @@
   ASSERT_TRUE(CInvok.getLangOpts()->AllowRecip);
 }
 
-TEST_F(CC1CommandLineGenerationTest, CanGenerateCC1CommandLineFlag) {
+TEST_F(CommandLineTest, CanGenerateCC1CommandLineFlag) {
   const char *Args[] = {"clang", "-xc++", "-fmodules-strict-context-hash", "-"};
 
-  CompilerInvocation CInvok;
   CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
 
   CInvok.generateCC1CommandLine(GeneratedArgs, *this);
@@ -87,11 +79,10 @@
   ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fmodules-strict-context-hash")));
 }
 
-TEST_F(CC1CommandLineGenerationTest, CanGenerateCC1CommandLineSeparate) {
+TEST_F(CommandLineTest, CanGenerateCC1CommandLineSeparate) {
   const char *TripleCStr = "i686-apple-darwin9";
   const char *Args[] = {"clang", "-xc++", "-triple", TripleCStr, "-"};
 
-  CompilerInvocation CInvok;
   CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
 
   CInvok.generateCC1CommandLine(GeneratedArgs, *this);
@@ -99,14 +90,12 @@
   ASSERT_THAT(GeneratedArgs, Contains(StrEq(TripleCStr)));
 }
 
-TEST_F(CC1CommandLineGenerationTest,
-   CanGenerateCC1CommandLineSeparateRequiredPresent) {
+TEST_F(CommandLineTest,  CanGenerateCC1CommandLineSeparateRequiredPresent) {
   const std::string DefaultTriple =
   llvm::Triple::normalize(llvm::sys::getDefaultTargetTriple());
   const char *Args[] = {"clang", "-xc++", "-triple", DefaultTriple.c_str(),
 "-"};
 
-  CompilerInvocation CInvok;
   CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
 
   CInvok.generateCC1CommandLine(GeneratedArgs, *this);
@@ -115,13 +104,11 @@
   ASSERT_THAT(GeneratedArgs, Contains(StrEq(DefaultTriple.c_str(;
 }
 
-TEST_F(CC1CommandLineGenerationTest,
-   CanGenerateCC1CommandLineSeparateRequiredAbsent) {
+TEST_F(CommandLineTest, CanGenerateCC1CommandLineSeparateRequiredAbsent) {
   const std::string DefaultTriple =
   llvm::Triple::normalize(llvm::sys::getDefaultTargetTriple());
   const char *Args[] = {"clang", "-xc++", "-"};
 
-  CompilerInvocation CInvok;
   CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
 
   CInvok.generateCC1CommandLine(GeneratedArgs, *this);
@@ -130,12 +117,11 @@
   ASSERT_THAT(GeneratedArgs, Contains(StrEq(DefaultTriple.c_str(;
 }
 
-TEST_F(CC1CommandLineGenerationTest, CanGenerateCC1CommandLineSeparateEnum) {
+TEST_F(CommandLineTest, CanGenerateCC1CommandLineSeparateEnum) {
   const char *RelocationModelCStr = "static";
   const char *Args[] = {"clang", "-xc++", "-mrelocation-model",
 RelocationModelCStr, "-"};
 
-  CompilerInvocation CInvok;
   CompilerInvocation::CreateFromArgs(CInvok, Args

[PATCH] D92826: [clang][cli] CompilerInvocationTest: rename member variable in fixture

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added a reviewer: dexonsmith.
Herald added a reviewer: a.sidorin.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Depends on D92825 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92826

Files:
  clang/unittests/Frontend/CompilerInvocationTest.cpp

Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -27,7 +27,7 @@
   IntrusiveRefCntPtr Diags;
   SmallVector GeneratedArgs;
   SmallVector GeneratedArgsStorage;
-  CompilerInvocation CInvok;
+  CompilerInvocation Invocation;
 
   const char *operator()(const Twine &Arg) {
 return GeneratedArgsStorage.emplace_back(Arg.str()).c_str();
@@ -40,41 +40,41 @@
 TEST_F(CommandLineTest, OptIsInitializedWithCustomDefaultValue) {
   const char *Args[] = {"clang", "-xc++"};
 
-  CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
-  ASSERT_TRUE(CInvok.getFrontendOpts().UseTemporary);
+  ASSERT_TRUE(Invocation.getFrontendOpts().UseTemporary);
 }
 
 TEST_F(CommandLineTest, OptOfNegativeFlagIsPopulatedWithFalse) {
   const char *Args[] = {"clang", "-xc++", "-fno-temp-file"};
 
-  CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
-  ASSERT_FALSE(CInvok.getFrontendOpts().UseTemporary);
+  ASSERT_FALSE(Invocation.getFrontendOpts().UseTemporary);
 }
 
 TEST_F(CommandLineTest, OptsOfImpliedPositiveFlagArePopulatedWithTrue) {
   const char *Args[] = {"clang", "-xc++", "-cl-unsafe-math-optimizations"};
 
-  CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
   // Explicitly provided flag.
-  ASSERT_TRUE(CInvok.getLangOpts()->CLUnsafeMath);
+  ASSERT_TRUE(Invocation.getLangOpts()->CLUnsafeMath);
 
   // Flags directly implied by explicitly provided flag.
-  ASSERT_TRUE(CInvok.getCodeGenOpts().LessPreciseFPMAD);
-  ASSERT_TRUE(CInvok.getLangOpts()->UnsafeFPMath);
+  ASSERT_TRUE(Invocation.getCodeGenOpts().LessPreciseFPMAD);
+  ASSERT_TRUE(Invocation.getLangOpts()->UnsafeFPMath);
 
   // Flag transitively implied by explicitly provided flag.
-  ASSERT_TRUE(CInvok.getLangOpts()->AllowRecip);
+  ASSERT_TRUE(Invocation.getLangOpts()->AllowRecip);
 }
 
 TEST_F(CommandLineTest, CanGenerateCC1CommandLineFlag) {
   const char *Args[] = {"clang", "-xc++", "-fmodules-strict-context-hash", "-"};
 
-  CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
-  CInvok.generateCC1CommandLine(GeneratedArgs, *this);
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
   ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fmodules-strict-context-hash")));
 }
@@ -83,9 +83,9 @@
   const char *TripleCStr = "i686-apple-darwin9";
   const char *Args[] = {"clang", "-xc++", "-triple", TripleCStr, "-"};
 
-  CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
-  CInvok.generateCC1CommandLine(GeneratedArgs, *this);
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
   ASSERT_THAT(GeneratedArgs, Contains(StrEq(TripleCStr)));
 }
@@ -96,9 +96,9 @@
   const char *Args[] = {"clang", "-xc++", "-triple", DefaultTriple.c_str(),
 "-"};
 
-  CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
-  CInvok.generateCC1CommandLine(GeneratedArgs, *this);
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
   // Triple should always be emitted even if it is the default
   ASSERT_THAT(GeneratedArgs, Contains(StrEq(DefaultTriple.c_str(;
@@ -109,9 +109,9 @@
   llvm::Triple::normalize(llvm::sys::getDefaultTargetTriple());
   const char *Args[] = {"clang", "-xc++", "-"};
 
-  CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
-  CInvok.generateCC1CommandLine(GeneratedArgs, *this);
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
   // Triple should always be emitted even if it is the default
   ASSERT_THAT(GeneratedArgs, Contains(StrEq(DefaultTriple.c_str(;
@@ -122,9 +122,9 @@
   const char *Args[] = {"clang", "-xc++", "-mrelocation-model",
 RelocationModelCStr, "-"};
 
-  CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
-  CInvok.generateCC1CommandLine(GeneratedArgs, *this);
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
   // Non default relocation model
   ASSERT_THAT(GeneratedArgs, C

[PATCH] D92827: [clang][cli] CompilerInvocationTest: split enum test into two

[Jan Svoboda via Phabricator via cfe-commits]

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

Depends on D92826 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92827

Files:
  clang/unittests/Frontend/CompilerInvocationTest.cpp


Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -17,9 +17,7 @@
 using namespace clang;
 
 using ::testing::Contains;
-using ::testing::Each;
 using ::testing::StrEq;
-using ::testing::StrNe;
 
 namespace {
 class CommandLineTest : public ::testing::Test {
@@ -117,27 +115,26 @@
   ASSERT_THAT(GeneratedArgs, Contains(StrEq(DefaultTriple.c_str(;
 }
 
-TEST_F(CommandLineTest, CanGenerateCC1CommandLineSeparateEnum) {
-  const char *RelocationModelCStr = "static";
-  const char *Args[] = {"clang", "-xc++", "-mrelocation-model",
-RelocationModelCStr, "-"};
+TEST_F(CommandLineTest, CanGenerateCC1CommandLineSeparateEnumNonDefault) {
+  const char *Args[] = {"clang", "-xc++", "-mrelocation-model", "static", "-"};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
-  // Non default relocation model
-  ASSERT_THAT(GeneratedArgs, Contains(StrEq(RelocationModelCStr)));
-  GeneratedArgs.clear();
+  // Non default relocation model.
+  ASSERT_THAT(GeneratedArgs, Contains(StrEq("static")));
+}
+
+TEST_F(CommandLineTest, CanGenerateCC1COmmandLineSeparateEnumDefault) {
+  const char *Args[] = {"clang", "-xc++", "-mrelocation-model", "pic", "-"};
 
-  RelocationModelCStr = "pic";
-  Args[3] = RelocationModelCStr;
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
-  CompilerInvocation Invocation2;
-  CompilerInvocation::CreateFromArgs(Invocation2, Args, *Diags);
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
-  Invocation2.generateCC1CommandLine(GeneratedArgs, *this);
-  ASSERT_THAT(GeneratedArgs, Each(StrNe(RelocationModelCStr)));
+  // Default relocation model.
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("pic";
 }
 
 TEST_F(CommandLineTest, NotPresentNegativeFlagNotGenerated) {


Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -17,9 +17,7 @@
 using namespace clang;
 
 using ::testing::Contains;
-using ::testing::Each;
 using ::testing::StrEq;
-using ::testing::StrNe;
 
 namespace {
 class CommandLineTest : public ::testing::Test {
@@ -117,27 +115,26 @@
   ASSERT_THAT(GeneratedArgs, Contains(StrEq(DefaultTriple.c_str(;
 }
 
-TEST_F(CommandLineTest, CanGenerateCC1CommandLineSeparateEnum) {
-  const char *RelocationModelCStr = "static";
-  const char *Args[] = {"clang", "-xc++", "-mrelocation-model",
-RelocationModelCStr, "-"};
+TEST_F(CommandLineTest, CanGenerateCC1CommandLineSeparateEnumNonDefault) {
+  const char *Args[] = {"clang", "-xc++", "-mrelocation-model", "static", "-"};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
-  // Non default relocation model
-  ASSERT_THAT(GeneratedArgs, Contains(StrEq(RelocationModelCStr)));
-  GeneratedArgs.clear();
+  // Non default relocation model.
+  ASSERT_THAT(GeneratedArgs, Contains(StrEq("static")));
+}
+
+TEST_F(CommandLineTest, CanGenerateCC1COmmandLineSeparateEnumDefault) {
+  const char *Args[] = {"clang", "-xc++", "-mrelocation-model", "pic", "-"};
 
-  RelocationModelCStr = "pic";
-  Args[3] = RelocationModelCStr;
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
-  CompilerInvocation Invocation2;
-  CompilerInvocation::CreateFromArgs(Invocation2, Args, *Diags);
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
-  Invocation2.generateCC1CommandLine(GeneratedArgs, *this);
-  ASSERT_THAT(GeneratedArgs, Each(StrNe(RelocationModelCStr)));
+  // Default relocation model.
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("pic";
 }
 
 TEST_F(CommandLineTest, NotPresentNegativeFlagNotGenerated) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92828: [clang][cli] CompilerInvocationTest: remove unnecessary command line arguments

[Jan Svoboda via Phabricator via cfe-commits]

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

Depends on D92827 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92828

Files:
  clang/unittests/Frontend/CompilerInvocationTest.cpp

Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -36,7 +36,7 @@
 };
 
 TEST_F(CommandLineTest, OptIsInitializedWithCustomDefaultValue) {
-  const char *Args[] = {"clang", "-xc++"};
+  const char *Args[] = {""};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
@@ -44,7 +44,7 @@
 }
 
 TEST_F(CommandLineTest, OptOfNegativeFlagIsPopulatedWithFalse) {
-  const char *Args[] = {"clang", "-xc++", "-fno-temp-file"};
+  const char *Args[] = {"-fno-temp-file"};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
@@ -52,7 +52,7 @@
 }
 
 TEST_F(CommandLineTest, OptsOfImpliedPositiveFlagArePopulatedWithTrue) {
-  const char *Args[] = {"clang", "-xc++", "-cl-unsafe-math-optimizations"};
+  const char *Args[] = {"-cl-unsafe-math-optimizations"};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
@@ -68,7 +68,7 @@
 }
 
 TEST_F(CommandLineTest, CanGenerateCC1CommandLineFlag) {
-  const char *Args[] = {"clang", "-xc++", "-fmodules-strict-context-hash", "-"};
+  const char *Args[] = {"-fmodules-strict-context-hash"};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
@@ -79,7 +79,7 @@
 
 TEST_F(CommandLineTest, CanGenerateCC1CommandLineSeparate) {
   const char *TripleCStr = "i686-apple-darwin9";
-  const char *Args[] = {"clang", "-xc++", "-triple", TripleCStr, "-"};
+  const char *Args[] = {"-triple", TripleCStr};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
@@ -91,8 +91,7 @@
 TEST_F(CommandLineTest,  CanGenerateCC1CommandLineSeparateRequiredPresent) {
   const std::string DefaultTriple =
   llvm::Triple::normalize(llvm::sys::getDefaultTargetTriple());
-  const char *Args[] = {"clang", "-xc++", "-triple", DefaultTriple.c_str(),
-"-"};
+  const char *Args[] = {"-triple", DefaultTriple.c_str()};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
@@ -105,7 +104,7 @@
 TEST_F(CommandLineTest, CanGenerateCC1CommandLineSeparateRequiredAbsent) {
   const std::string DefaultTriple =
   llvm::Triple::normalize(llvm::sys::getDefaultTargetTriple());
-  const char *Args[] = {"clang", "-xc++", "-"};
+  const char *Args[] = {""};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
@@ -116,7 +115,7 @@
 }
 
 TEST_F(CommandLineTest, CanGenerateCC1CommandLineSeparateEnumNonDefault) {
-  const char *Args[] = {"clang", "-xc++", "-mrelocation-model", "static", "-"};
+  const char *Args[] = {"-mrelocation-model", "static"};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
@@ -127,7 +126,7 @@
 }
 
 TEST_F(CommandLineTest, CanGenerateCC1COmmandLineSeparateEnumDefault) {
-  const char *Args[] = {"clang", "-xc++", "-mrelocation-model", "pic", "-"};
+  const char *Args[] = {"-mrelocation-model", "pic"};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
@@ -138,7 +137,7 @@
 }
 
 TEST_F(CommandLineTest, NotPresentNegativeFlagNotGenerated) {
-  const char *Args[] = {"clang", "-xc++"};
+  const char *Args[] = {""};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
@@ -148,7 +147,7 @@
 }
 
 TEST_F(CommandLineTest, PresentNegativeFlagGenerated) {
-  const char *Args[] = {"clang", "-xc++", "-fno-temp-file"};
+  const char *Args[] = {"-fno-temp-file"};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
@@ -158,7 +157,7 @@
 }
 
 TEST_F(CommandLineTest, NotPresentAndNotImpliedNotGenerated) {
-  const char *Args[] = {"clang", "-xc++"};
+  const char *Args[] = {""};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
@@ -172,7 +171,7 @@
 }
 
 TEST_F(CommandLineTest, NotPresentAndImpliedNotGenerated) {
-  const char *Args[] = {"clang", "-xc++", "-cl-unsafe-math-optimizations"};
+  const char *Args[] = {"-cl-unsafe-math-optimizations"};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
@@ -185,8 +184,8 @@
 }
 
 TEST_F(CommandLineTest, PresentAndImpliedNotGenerated) {
-  const char *Args[] = {"clang", "-xc++", "-cl-unsafe-math-optimizations",
-"-cl-mad-enable", "-menable-unsafe-fp-math"};
+  const char *Args[] = {"-cl-unsafe-math-optimizations", "-cl-mad-enable",
+"-menable-unsafe-fp-math"};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
@@ -199,8 +198,7 @@
 }
 
 TEST_F(CommandLineTest, PresentAndNotImpliedGenerated) {
-  const char *Args[] = {"clang", "-xc+

[PATCH] D92829: [clang][cli] CompilerInvocationTest: check arg parsing does not produce diagnostics

[Jan Svoboda via Phabricator via cfe-commits]

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

Depends on D92828 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92829

Files:
  clang/unittests/Frontend/CompilerInvocationTest.cpp

Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -8,6 +8,7 @@
 
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/TextDiagnosticBuffer.h"
 #include "llvm/Support/Host.h"
 
 #include "gmock/gmock.h"
@@ -32,7 +33,9 @@
   }
 
   CommandLineTest()
-  : Diags(CompilerInstance::createDiagnostics(new DiagnosticOptions())) {}
+  : Diags(CompilerInstance::createDiagnostics(new DiagnosticOptions(),
+  new TextDiagnosticBuffer())) {
+  }
 };
 
 TEST_F(CommandLineTest, OptIsInitializedWithCustomDefaultValue) {
@@ -40,6 +43,8 @@
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+
   ASSERT_TRUE(Invocation.getFrontendOpts().UseTemporary);
 }
 
@@ -48,6 +53,8 @@
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+
   ASSERT_FALSE(Invocation.getFrontendOpts().UseTemporary);
 }
 
@@ -56,6 +63,8 @@
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+
   // Explicitly provided flag.
   ASSERT_TRUE(Invocation.getLangOpts()->CLUnsafeMath);
 
@@ -72,6 +81,8 @@
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
   ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fmodules-strict-context-hash")));
@@ -83,6 +94,8 @@
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
   ASSERT_THAT(GeneratedArgs, Contains(StrEq(TripleCStr)));
@@ -95,6 +108,8 @@
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
   // Triple should always be emitted even if it is the default
@@ -108,6 +123,8 @@
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
   // Triple should always be emitted even if it is the default
@@ -119,6 +136,8 @@
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
   // Non default relocation model.
@@ -130,6 +149,8 @@
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
   // Default relocation model.
@@ -141,6 +162,8 @@
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
   ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fno-temp-file";
@@ -151,6 +174,8 @@
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
   ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fno-temp-file")));
@@ -161,6 +186,8 @@
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
   // Missing options are not generated.
@@ -175,6 +202,8 @@
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
   // Missing options that were implied are not generated.
@@ -189,6 +218,8 @@
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
   // Present options that were also implied are not generated.
@@ -202,6 +233,8 @@
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
   // Present options that were not implied are generated.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.

[PATCH] D92830: [clang][cli] CompilerInvocationTest: join and add test cases

[Jan Svoboda via Phabricator via cfe-commits]

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

Depends on D92829 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92830

Files:
  clang/unittests/Frontend/CompilerInvocationTest.cpp

Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -38,42 +38,43 @@
   }
 };
 
-TEST_F(CommandLineTest, OptIsInitializedWithCustomDefaultValue) {
+// Boolean option with a keypath that defaults to true.
+// The only flag with a negative spelling can set the keypath to false.
+
+TEST_F(CommandLineTest, BoolOptionDefaultTrueSingleFlagNotPresent) {
   const char *Args[] = {""};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
   ASSERT_FALSE(Diags->hasErrorOccurred());
-
   ASSERT_TRUE(Invocation.getFrontendOpts().UseTemporary);
+
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fno-temp-file";
 }
 
-TEST_F(CommandLineTest, OptOfNegativeFlagIsPopulatedWithFalse) {
+TEST_F(CommandLineTest, BoolOptionDefaultTrueSingleFlagPresent) {
   const char *Args[] = {"-fno-temp-file"};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
   ASSERT_FALSE(Diags->hasErrorOccurred());
-
   ASSERT_FALSE(Invocation.getFrontendOpts().UseTemporary);
-}
 
-TEST_F(CommandLineTest, OptsOfImpliedPositiveFlagArePopulatedWithTrue) {
-  const char *Args[] = {"-cl-unsafe-math-optimizations"};
-
-  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
-  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fno-temp-file")));
+}
 
-  // Explicitly provided flag.
-  ASSERT_TRUE(Invocation.getLangOpts()->CLUnsafeMath);
+TEST_F(CommandLineTest, BoolOptionDefaultTrueSingleFlagUnknownPresent) {
+  const char *Args[] = {"-ftemp-file"};
 
-  // Flags directly implied by explicitly provided flag.
-  ASSERT_TRUE(Invocation.getCodeGenOpts().LessPreciseFPMAD);
-  ASSERT_TRUE(Invocation.getLangOpts()->UnsafeFPMath);
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
-  // Flag transitively implied by explicitly provided flag.
-  ASSERT_TRUE(Invocation.getLangOpts()->AllowRecip);
+  // Driver-only flag.
+  ASSERT_TRUE(Diags->hasErrorOccurred());
+  ASSERT_TRUE(Invocation.getFrontendOpts().UseTemporary);
 }
 
 TEST_F(CommandLineTest, CanGenerateCC1CommandLineFlag) {
@@ -157,75 +158,101 @@
   ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("pic";
 }
 
-TEST_F(CommandLineTest, NotPresentNegativeFlagNotGenerated) {
-  const char *Args[] = {""};
-
-  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
-
-  ASSERT_FALSE(Diags->hasErrorOccurred());
-
-  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
-
-  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fno-temp-file";
-}
-
-TEST_F(CommandLineTest, PresentNegativeFlagGenerated) {
-  const char *Args[] = {"-fno-temp-file"};
-
-  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
-
-  ASSERT_FALSE(Diags->hasErrorOccurred());
-
-  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
-
-  ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fno-temp-file")));
-}
+// Tree of boolean options that can be (directly or transitively) implied by
+// their parent:
+//
+//   * -cl-unsafe-math-optimizations
+// * -cl-mad-enable
+// * -menable-unsafe-fp-math
+//   * -freciprocal-math
 
-TEST_F(CommandLineTest, NotPresentAndNotImpliedNotGenerated) {
+TEST_F(CommandLineTest, ImpliedBoolOptionsNoFlagPresent) {
   const char *Args[] = {""};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
   ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_FALSE(Invocation.getLangOpts()->CLUnsafeMath);
+  ASSERT_FALSE(Invocation.getCodeGenOpts().LessPreciseFPMAD);
+  ASSERT_FALSE(Invocation.getLangOpts()->UnsafeFPMath);
+  ASSERT_FALSE(Invocation.getLangOpts()->AllowRecip);
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
-  // Missing options are not generated.
+  // Not generated - missing.
   ASSERT_THAT(GeneratedArgs,
   Not(Contains(StrEq("-cl-unsafe-math-optimizations";
   ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-cl-mad-enable";
   ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-menable-unsafe-fp-math";
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-freciprocal-math";
 }
 
-TEST_F(CommandLineTest, NotPresentAndImpliedNotGenerated) {
+TEST_F(CommandLineTest, ImpliedBoolOptionsRootFlagPresent) {
   const char *Args[] = {"-cl-unsafe-math-optimizations"};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
   ASSERT_FALSE

[PATCH] D92774: [clang][cli] Add command line marshalling tests

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 310134.
jansvoboda11 added a comment.

Split into multiple patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92774

Files:
  clang/unittests/Frontend/CompilerInvocationTest.cpp

Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -77,6 +77,150 @@
   ASSERT_TRUE(Invocation.getFrontendOpts().UseTemporary);
 }
 
+// Boolean option with a keypath that defaults to true.
+// The flag with negative spelling can set the keypath to false.
+// The flag with positive spelling can reset the keypath to true.
+
+TEST_F(CommandLineTest, BoolOptionDefaultTruePresentNone) {
+  const char *Args[] = {""};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_TRUE(Invocation.getCodeGenOpts().Autolink);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultTruePresentNegChange) {
+  const char *Args[] = {"-fno-autolink"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_FALSE(Invocation.getCodeGenOpts().Autolink);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultTruePresentPosReset) {
+  const char *Args[] = {"-fautolink"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_TRUE(Diags->hasErrorOccurred()); // Driver-only flag.
+  ASSERT_TRUE(Invocation.getCodeGenOpts().Autolink);
+}
+
+// Boolean option with a keypath that defaults to false.
+// The flag with negative spelling can set the keypath to true.
+// The flag with positive spelling can reset the keypath to false.
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentNone) {
+  const char *Args[] = {""};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_FALSE(Invocation.getCodeGenOpts().NoInlineLineTables);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentNegChange) {
+  const char *Args[] = {"-gno-inline-line-tables"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_TRUE(Invocation.getCodeGenOpts().NoInlineLineTables);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentPosReset) {
+  const char *Args[] = {"-ginline-line-tables"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_TRUE(Diags->hasErrorOccurred()); // Driver-only flag.
+  ASSERT_TRUE(Invocation.getCodeGenOpts().Autolink);
+}
+
+// Boolean option with a keypath that defaults to false.
+// The flag with positive spelling can set the keypath to true.
+// The flag with negative spelling can reset the keypath to false.
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentNoneX) {
+  const char *Args[] = {""};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_FALSE(Invocation.getCodeGenOpts().CodeViewGHash);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentPosChange) {
+  const char *Args[] = {"-gcodeview-ghash"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_TRUE(Invocation.getCodeGenOpts().CodeViewGHash);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentNegReset) {
+  const char *Args[] = {"-gno-codeview-ghash"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_TRUE(Diags->hasErrorOccurred()); // Driver-only flag.
+  ASSERT_FALSE(Invocation.getCodeGenOpts().CodeViewGHash);
+}
+
+// Boolean option with a keypath that defaults to an arbitrary expression.
+// The flag with positive spelling can set the keypath to true.
+// The flag with negative spelling can set the keypath to false.
+
+// NOTE: The following tests need to be updated when we start enabling the new
+// pass manager by default.
+
+TEST_F(CommandLineTest, BoolOptionDefaultArbitraryTwoFlagsPresentNone) {
+  const char *Args = {""};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_EQ(Invocation.getCodeGenOpts().ExperimentalNewPassManager, false);
+
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+  ASSERT_THAT(GeneratedArgs,
+  Not(Contains(StrEq("-fexperimental-new-pass-manager";
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultArbitraryTwoFlagsPresentPos) {
+  const char *Args[] = {"-fexperimental-new-pass-manager"};
+
+  CompilerInvocation::CreateFromArgs(Invoca

[PATCH] D92812: [X86] AMD Znver3 (Family 19H) Enablement

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

This patch doesn't look like it is taking into account the existing (albeit 
limited) support for znver3 - it looks like a very bad merge imo.

It'd make much more sense to split this into several smaller incremental 
patches:

1 - cleanup any missing basic znver3 handling, building on the work that 
@bkramer has already done
2 - patches for the new INVLPGB/TLBSYNC/SNP instructions
3 - new znver3 scheduler model


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92812

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

[PATCH] D92298: [AST][RecoveryAST] Preserve type for member call expr if argments are not matched.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

The thrust of this change is great, I'm just having trouble being sure there 
are no bad side-effects.

---

I almost left a comment here saying we might want to recover here... before 
noticing that we recover all the way up at ParsePostfixExpressionSuffix().

It may be worth a comment on the function implementation that RecoveryExpr 
fallback is done at a higher level.
But on this note, I think we have to consider the effect on *other* callsites.
For example `Sema::tryExprAsCall` checks if the result is non-null in order to 
produce "function must be callled, did you mean to call with no arguments"... 
but now I think we're returning non-null even if args are required. The fix 
there is to check containsErrors too.

There are a bunch of "internal" uses to Sema::BuildCallExpr (which calls this 
function), like coroutines, decomposition, pseudo-object etc that might need to 
be checked.

Finally BuildRecoveryCallExpr in SemaOverload itself calls BuildCallExpr, and 
maybe we can end up with a RecoveryExpr wrapping a RecoveryExpr.

---

I'm not sure what the most principled thing to do here is. Recovering (only) at 
a higher level seems sensible, but then we lack a way to propagate the type. 
Checking for containsErrors() where needed seems fragile. Adding a new state to 
ExprResult (null/error/Expr/QualType) is a pretty big change that may not be 
useful in general.

I'm not opposed to this patch if we carefully audit all callers but this 
doesn't feel great. Maybe an ugly "allowRecovery" flag to BuildCallExpr is the 
right compromise, at least it forces us to be explicit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92298

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

[PATCH] D92775: [clang][cli] Add flexible TableGen multiclass for boolean options

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 310135.
jansvoboda11 added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92775

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td

Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -171,11 +171,12 @@
   code ValueExtractor = "(extractMaskValue)";
 }
 
-class MarshallingInfoBooleanFlag
+// Marshalling info for booleans. Applied to the flag setting keypath to false.
+class MarshallingInfoBooleanFlag
   : MarshallingInfoFlag {
-  bit ShouldAlwaysEmit = 1;
-  code Normalizer = "makeBooleanFlagNormalizer(OPT_"#neg_name#")";
-  code Denormalizer = "makeBooleanFlagDenormalizer(\""#neg_spelling#"\")";
+  code Normalizer = "makeBooleanOptionNormalizer("#value#", "#other_value#", OPT_"#other_name#")";
+  code Denormalizer = "makeBooleanOptionDenormalizer("#value#",\""#other_spelling#"\")";
 }
 
 // Mixins for additional marshalling attributes.
Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -88,7 +88,9 @@
   ASSERT_FALSE(Diags->hasErrorOccurred());
   ASSERT_TRUE(Invocation.getCodeGenOpts().Autolink);
 
-  // TODO: Test argument generation.
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fautolink";
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fno-autolink";
 }
 
 TEST_F(CommandLineTest, BoolOptionDefaultTruePresentNegChange) {
@@ -98,7 +100,9 @@
   ASSERT_FALSE(Diags->hasErrorOccurred());
   ASSERT_FALSE(Invocation.getCodeGenOpts().Autolink);
 
-  // TODO: Test argument generation.
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+  ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fno-autolink")));
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fautolink";
 }
 
 TEST_F(CommandLineTest, BoolOptionDefaultTruePresentPosReset) {
@@ -120,7 +124,9 @@
   ASSERT_FALSE(Diags->hasErrorOccurred());
   ASSERT_FALSE(Invocation.getCodeGenOpts().NoInlineLineTables);
 
-  // TODO: Test argument generation.
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-ginline-line-tables";
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-gno-inline-line-tables";
 }
 
 TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentNegChange) {
@@ -130,7 +136,9 @@
   ASSERT_FALSE(Diags->hasErrorOccurred());
   ASSERT_TRUE(Invocation.getCodeGenOpts().NoInlineLineTables);
 
-  // TODO: Test argument generation.
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+  ASSERT_THAT(GeneratedArgs, Contains(StrEq("-gno-inline-line-tables")));
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-ginline-line-tables";
 }
 
 TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentPosReset) {
@@ -152,7 +160,9 @@
   ASSERT_FALSE(Diags->hasErrorOccurred());
   ASSERT_FALSE(Invocation.getCodeGenOpts().CodeViewGHash);
 
-  // TODO: Test argument generation.
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-gcodeview-ghash";
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-gno-codeview-ghash";
 }
 
 TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentPosChange) {
@@ -162,7 +172,9 @@
   ASSERT_FALSE(Diags->hasErrorOccurred());
   ASSERT_TRUE(Invocation.getCodeGenOpts().CodeViewGHash);
 
-  // TODO: Test argument generation.
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+  ASSERT_THAT(GeneratedArgs, Contains(StrEq("-gcodeview-ghash")));
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-gno-codeview-ghash";
 }
 
 TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentNegReset) {
@@ -216,7 +228,7 @@
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
   ASSERT_THAT(GeneratedArgs,
-  Contains(StrEq("-fno-experimental-new-pass-manager")));
+  Not(Contains(StrEq("-fno-experimental-new-pass-manager";
   ASSERT_THAT(GeneratedArgs,
   Not(Contains(StrEq("-fexperimental-new-pass-manager";
 }
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -185,20 +185,24 @@
   return FlagToValueNormalizer{std::move(Value)};
 }
 
-static auto makeBooleanFlagNormalizer(OptSpecifier NegOpt) {
-  return [NegOpt](OptSpecifier PosOpt, unsigned, const ArgList &Args,
-  DiagnosticsEngine &) -> Optional {
-   

[PATCH] D92774: [clang][cli] Add command line marshalling tests

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

I'm sorry about that, I understand that was hard to review.

This is now split this into multiple patches: D92825 
, D92826 , 
D92827 , D92828 
, D92829 , 
D92830 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92774

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

[PATCH] D92181: [clangd] NFC: Add client-side logging for remote index requests

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/index/remote/Client.cpp:45
 Context.AddMetadata("version", clang::getClangToolFullVersion("clangd"));
-std::chrono::system_clock::time_point Deadline =
-std::chrono::system_clock::now() + DeadlineWaitingTime;
+const std::chrono::system_clock::time_point StartTime =
+std::chrono::system_clock::now();

(we don't ussually use `const` for locals)



Comment at: clang-tools-extra/clangd/index/remote/Client.cpp:50
 auto Reader = (Stub.get()->*RPCCall)(&Context, RPCRequest);
+vlog("Sending RPC Request {0}: {1}", RequestT::descriptor()->name(),
+ RPCRequest.DebugString());

Including the request payload is IMO too much even at --log=verbose, we do this 
for LSP messages but those are more useful/central. Vlog the request and dlog 
the payload? (Similar to log/vlog for LSP)



Comment at: clang-tools-extra/clangd/index/remote/Client.cpp:74
+.count();
+vlog("RPC Request {0} => OK: {1} results in {2}ms",
+ RequestT::descriptor()->name(), Successful, Millis);

"RPC Request" may be a bit jargony for these messages. And "Request" is part of 
the RequestT name.
It'd be nice to include the server name too, there may be several.

What about `Remote index: LookupRequest => 123 results in 18ms. 
[llvm.clangd-index-staging.org:1234]`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92181

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

[PATCH] D92771: [analyzer][StdLibraryFunctionsChecker] Add more return value contraints

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

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

It must have been a tedious task to collect all these - without any copy-paste 
errors, really impressive!

It's good to go, however, if you don't mind there would be some readability 
issues yet to solve in a later path in the inline comments.




Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1367-1368
+.Case(ReturnsZeroOrMinusOne)
 .ArgConstraint(ArgumentCondition(
 0, WithinRange, Range(0, IntMax;
 

I think you should hoist this ArgumentCondition construction with a lambda 
call. It would lead to more readable summaries.

```lang=c++
const auto ValidFileDescriptorArgAt = [](unsigned ArgIdx) {
  return ArgumentCondition(ArgIdx, WithinRange, Range(0, IntMax;
};
```
Probably the very same principle would apply for handling `off_t` arguments.

You can probably find a better name, but you get the idea.
If you agree on this, you could create a follow-up patch.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1888
+Summary(NoEvalCall)
+.Case({ReturnValueCondition(WithinRange, SingleValue(-1))})
+.ArgConstraint(NotNull(ArgNo(0;

The same principle applies here too.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1905
 Summary(NoEvalCall)
+.Case({ReturnValueCondition(WithinRange, Range(-1, 
UCharRangeMax))})
 .ArgConstraint(ArgumentCondition(0, WithinRange, Range(0, IntMax)))

It's a sane overapproximation. Perfectly fine for us.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92771

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

[PATCH] D92771: [analyzer][StdLibraryFunctionsChecker] Add more return value contraints

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

steakhal added a comment.

One more thing.
Please reflow the path's summary, to keep the range constraint in a single line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92771

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

[PATCH] D92775: [clang][cli] Add flexible TableGen multiclass for boolean options

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

That's great to hear, thanks.




Comment at: clang/include/clang/Driver/Options.td:371
+  // TODO: Assert that the flags have different value.
+  // TODO: Assert that only one of the flags can be implied.
+

Does TableGen support some kind of assertion mechanism?



Comment at: clang/unittests/Frontend/CompilerInvocationTest.cpp:226
   ASSERT_THAT(GeneratedArgs,
-  Contains(StrEq("-fno-experimental-new-pass-manager")));
+  Not(Contains(StrEq("-fno-experimental-new-pass-manager";
   ASSERT_THAT(GeneratedArgs,

dexonsmith wrote:
> Can you clarify why this was dropped? Was it previously emitted due to a 
> limitation in the implementation, or are we no longer supporting options that 
> always get emitted for clarity?
This option was the only one using the old infrastructure 
(`BooleanMarshalledFFlag`).
It was set up to always generate the flag, even when it wasn't necessary (this 
flag sets the keypath to its default value).

I think we should aim to generate only command line arguments that are 
necessary to preserve the original invocation semantics.
I imagine this will be useful when debugging: one won't need to scan hundreds 
of flags that don't have any effect on CompilerInvocation.



Comment at: llvm/include/llvm/Option/OptParser.td:176
   : MarshallingInfoFlag {
-  bit ShouldAlwaysEmit = 1;
-  code Normalizer = "makeBooleanFlagNormalizer(OPT_"#neg_name#")";

Removal of this property causes the disappearance of 
`-fno-experimental-new-pass-manager` from list of generated arguments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92775

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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added a comment.

This change looks fine to me, but I'm slightly concerned about 
https://reviews.llvm.org/D85788 - see my last comment on that revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added a comment.

In D81678#2438264 , @eugenis wrote:

> This change looks fine to me, but I'm slightly concerned about 
> https://reviews.llvm.org/D85788 - see my last comment on that revision.

Thank you for the link! I left a comment there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

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

[PATCH] D91317: Support: Add RedirectingFileSystem::create from simple list of redirections

[Jan Svoboda via Phabricator via cfe-commits]

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

LGTM.


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

https://reviews.llvm.org/D91317

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

[PATCH] D92630: ARCMigrate: Use hash_combine in the DenseMapInfo for EditEntry

[Jan Svoboda via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92630

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

[PATCH] D92773: [clang][cli] Unify boolean marshalling

[Jan Svoboda 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 rG083e035c47f6: [clang][cli] Unify boolean marshalling 
(authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92773

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -63,9 +63,7 @@
 
 class MarshallingInfo {
 public:
-  using Ptr = std::unique_ptr;
-
-  const char *MacroName;
+  static constexpr const char *MacroName = "OPTION_WITH_MARSHALLING";
   const Record &R;
   bool ShouldAlwaysEmit;
   StringRef KeyPath;
@@ -99,14 +97,9 @@
   static constexpr const char *ValueTablesDecl =
   "static const SimpleEnumValueTable SimpleEnumValueTables[] = ";
 
-  MarshallingInfo(const Record &R)
-  : MacroName("OPTION_WITH_MARSHALLING"), R(R) {}
-  MarshallingInfo(const char *MacroName, const Record &R)
-  : MacroName(MacroName), R(R){};
-
-  virtual ~MarshallingInfo() = default;
+  MarshallingInfo(const Record &R) : R(R) {}
 
-  virtual void emit(raw_ostream &OS) const {
+  void emit(raw_ostream &OS) const {
 write_cstring(OS, StringRef(getOptionSpelling(R)));
 OS << ", ";
 OS << ShouldAlwaysEmit;
@@ -157,59 +150,35 @@
 
 size_t MarshallingInfo::NextTableIndex = 0;
 
-class MarshallingInfoBooleanFlag : public MarshallingInfo {
-public:
-  const Record &NegOption;
-
-  MarshallingInfoBooleanFlag(const Record &Option, const Record &NegOption)
-  : MarshallingInfo("OPTION_WITH_MARSHALLING_BOOLEAN", Option),
-NegOption(NegOption) {}
-
-  void emit(raw_ostream &OS) const override {
-MarshallingInfo::emit(OS);
-OS << ", ";
-OS << getOptionName(NegOption);
-OS << ", ";
-write_cstring(OS, getOptionSpelling(NegOption));
-  }
-};
-
-static MarshallingInfo::Ptr createMarshallingInfo(const Record &R) {
+static MarshallingInfo createMarshallingInfo(const Record &R) {
   assert(!isa(R.getValueInit("KeyPath")) &&
  !isa(R.getValueInit("DefaultValue")) &&
  !isa(R.getValueInit("ValueMerger")) &&
  "MarshallingInfo must have a provide a keypath, default value and a "
  "value merger");
 
-  MarshallingInfo::Ptr Ret;
-  StringRef KindString = R.getValueAsString("MarshallingInfoKind");
-  if (KindString == "Default") {
-Ret = std::make_unique(R);
-  } else if (KindString == "BooleanFlag") {
-Ret = std::make_unique(
-R, *R.getValueAsDef("NegOption"));
-  }
+  MarshallingInfo Ret(R);
 
-  Ret->ShouldAlwaysEmit = R.getValueAsBit("ShouldAlwaysEmit");
-  Ret->KeyPath = R.getValueAsString("KeyPath");
-  Ret->DefaultValue = R.getValueAsString("DefaultValue");
-  Ret->NormalizedValuesScope = R.getValueAsString("NormalizedValuesScope");
-  Ret->ImpliedCheck = R.getValueAsString("ImpliedCheck");
-  Ret->ImpliedValue =
-  R.getValueAsOptionalString("ImpliedValue").getValueOr(Ret->DefaultValue);
+  Ret.ShouldAlwaysEmit = R.getValueAsBit("ShouldAlwaysEmit");
+  Ret.KeyPath = R.getValueAsString("KeyPath");
+  Ret.DefaultValue = R.getValueAsString("DefaultValue");
+  Ret.NormalizedValuesScope = R.getValueAsString("NormalizedValuesScope");
+  Ret.ImpliedCheck = R.getValueAsString("ImpliedCheck");
+  Ret.ImpliedValue =
+  R.getValueAsOptionalString("ImpliedValue").getValueOr(Ret.DefaultValue);
 
-  Ret->Normalizer = R.getValueAsString("Normalizer");
-  Ret->Denormalizer = R.getValueAsString("Denormalizer");
-  Ret->ValueMerger = R.getValueAsString("ValueMerger");
-  Ret->ValueExtractor = R.getValueAsString("ValueExtractor");
+  Ret.Normalizer = R.getValueAsString("Normalizer");
+  Ret.Denormalizer = R.getValueAsString("Denormalizer");
+  Ret.ValueMerger = R.getValueAsString("ValueMerger");
+  Ret.ValueExtractor = R.getValueAsString("ValueExtractor");
 
   if (!isa(R.getValueInit("NormalizedValues"))) {
 assert(!isa(R.getValueInit("Values")) &&
"Cannot provide normalized values for value-less options");
-Ret->TableIndex = MarshallingInfo::NextTableIndex++;
-Ret->NormalizedValues = R.getValueAsListOfStrings("NormalizedValues");
-Ret->Values.reserve(Ret->NormalizedValues.size());
-Ret->ValueTableName = getOptionName(R) + "ValueTable";
+Ret.TableIndex = MarshallingInfo::NextTableIndex++;
+Ret.NormalizedValues = R.getValueAsListOfStrings("NormalizedValues");
+Ret.Values.reserve(Ret.NormalizedValues.size());
+Ret.ValueTableName = getOptionName(R) + "ValueTable";
 
 StringRef ValuesStr = R.getValueAsString("Values");
 for (;;) {
@@ -217,22 +186,18 @@
   if (Idx == StringRef::npos)
 break;
   if (Idx > 0

[clang] 083e035 - [clang][cli] Unify boolean marshalling

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2020-12-08T13:47:30+01:00
New Revision: 083e035c47f6c73084ecf5ab7f41cddca19ce332

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

LOG: [clang][cli] Unify boolean marshalling

Use lambdas with captures to replace the redundant infrastructure for 
marshalling of two boolean flags that control the same keypath.

Reviewed By: dexonsmith

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

Added: 


Modified: 
clang/include/clang/Driver/Options.td
clang/lib/Frontend/CompilerInvocation.cpp
llvm/include/llvm/Option/OptParser.td
llvm/utils/TableGen/OptParserEmitter.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index c6159f50b781..794aa24f997d 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -271,7 +271,7 @@ multiclass OptOutFFlag {
   def fno_#NAME : Flag<["-"], "fno-"#name>, HelpText;
   def f#NAME : Flag<["-"], "f"#name>, HelpText,
-MarshallingInfoBooleanFlag("fno_"#NAME)>;
+MarshallingInfoBooleanFlag;
 }
 
 /

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index e31f6aa34b36..547dadd37931 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -185,25 +185,21 @@ static FlagToValueNormalizer 
makeFlagToValueNormalizer(T Value) {
   return FlagToValueNormalizer{std::move(Value)};
 }
 
-static Optional normalizeBooleanFlag(OptSpecifier PosOpt,
-   OptSpecifier NegOpt,
-   unsigned TableIndex,
-   const ArgList &Args,
-   DiagnosticsEngine &Diags) {
-  if (const Arg *A = Args.getLastArg(PosOpt, NegOpt))
-return A->getOption().matches(PosOpt);
-  return None;
+static auto makeBooleanFlagNormalizer(OptSpecifier NegOpt) {
+  return [NegOpt](OptSpecifier PosOpt, unsigned, const ArgList &Args,
+  DiagnosticsEngine &) -> Optional {
+if (const Arg *A = Args.getLastArg(PosOpt, NegOpt))
+  return A->getOption().matches(PosOpt);
+return None;
+  };
 }
 
-static void denormalizeBooleanFlag(SmallVectorImpl &Args,
-   const char *Spelling,
-   const char *NegSpelling,
-   CompilerInvocation::StringAllocator SA,
-   unsigned TableIndex, unsigned Value) {
-  if (Value)
-Args.push_back(Spelling);
-  else
-Args.push_back(NegSpelling);
+static auto makeBooleanFlagDenormalizer(const char *NegSpelling) {
+  return [NegSpelling](
+ SmallVectorImpl &Args, const char *PosSpelling,
+ CompilerInvocation::StringAllocator, unsigned, unsigned Value) {
+Args.push_back(Value ? PosSpelling : NegSpelling);
+  };
 }
 
 static Optional
@@ -3779,23 +3775,7 @@ bool CompilerInvocation::parseSimpleArgs(const ArgList 
&Args,
   this->KEYPATH, static_castKEYPATH)>(*MaybeValue));   
\
   }
 
-#define OPTION_WITH_MARSHALLING_BOOLEAN(   
\
-PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,
\
-HELPTEXT, METAVAR, VALUES, SPELLING, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE,  
\
-IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER, MERGER, EXTRACTOR, 
\
-TABLE_INDEX, NEG_ID, NEG_SPELLING) 
\
-  {
\
-this->KEYPATH = MERGER(this->KEYPATH, DEFAULT_VALUE);  
\
-if (IMPLIED_CHECK) 
\
-  this->KEYPATH = MERGER(this->KEYPATH, IMPLIED_VALUE);
\
-if (auto MaybeValue =  
\
-NORMALIZER(OPT_##ID, OPT_##NEG_ID, TABLE_INDEX, Args, Diags))  
\
-  this->KEYPATH = MERGER(  
\
-  this->KEYPATH, static_castKEYPATH)>(*MaybeValue));   
\
-  }
-
 #include "clang/Driver/Options.inc"
-#undef OPTION_WITH_MARSHALLING_BOOLEAN
 #undef OPTION_WITH_MARSHALLING
   return true;
 }
@@ -4060,20 +4040,7 @@ void CompilerInvocation::generateCC1CommandLine(
 }(EXTRACTOR(this->KEYPATH));   
\
   }
 
-#define OPTION_WITH_MARSHALLING_BOOLEAN(   
\
-PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,
\
-HELPTEXT, METAVAR, VALUES, SPELLING, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE,  
\
-IMPLIED_CHECK, IMPLIED_

[PATCH] D92837: [X86] Support tilezero intrinsic and c interface for AMX.

[LuoYuanke via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92837

Files:
  clang/include/clang/Basic/BuiltinsX86_64.def
  clang/lib/Headers/amxintrin.h
  clang/test/CodeGen/X86/amx_api.c
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/lib/Target/X86/X86ExpandPseudo.cpp
  llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
  llvm/lib/Target/X86/X86InstrAMX.td
  llvm/lib/Target/X86/X86PreTileConfig.cpp
  llvm/lib/Target/X86/X86RegisterInfo.cpp
  llvm/lib/Target/X86/X86RegisterInfo.td
  llvm/test/CodeGen/X86/AMX/amx-tile-basic.ll

Index: llvm/test/CodeGen/X86/AMX/amx-tile-basic.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/AMX/amx-tile-basic.ll
@@ -0,0 +1,35 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+amx-tile -verify-machineinstrs | FileCheck %s
+
+define void @test_amx(i8* %pointer, i8* %base, i64 %stride) {
+; CHECK-LABEL: test_amx:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:movb $1, -{{[0-9]+}}(%rsp)
+; CHECK-NEXT:movb $8, -{{[0-9]+}}(%rsp)
+; CHECK-NEXT:movw $8, -{{[0-9]+}}(%rsp)
+; CHECK-NEXT:movb $8, -{{[0-9]+}}(%rsp)
+; CHECK-NEXT:movw $8, -{{[0-9]+}}(%rsp)
+; CHECK-NEXT:movb $8, -{{[0-9]+}}(%rsp)
+; CHECK-NEXT:movw $8, -{{[0-9]+}}(%rsp)
+; CHECK-NEXT:ldtilecfg -{{[0-9]+}}(%rsp)
+; CHECK-NEXT:movw $8, %ax
+; CHECK-NEXT:tilezero %tmm0
+; CHECK-NEXT:tileloadd (%rsi,%rdx), %tmm1
+; CHECK-NEXT:tileloadd (%rsi,%rdx), %tmm2
+; CHECK-NEXT:tdpbssd %tmm2, %tmm1, %tmm0
+; CHECK-NEXT:tilestored %tmm0, (%rdi,%rdx)
+; CHECK-NEXT:tilerelease
+; CHECK-NEXT:retq
+  %c = call x86_amx @llvm.x86.tilezero.internal(i16 8, i16 8)
+  %a = call x86_amx @llvm.x86.tileloadd64.internal(i16 8, i16 8, i8* %base, i64 %stride)
+  %b = call x86_amx @llvm.x86.tileloadd64.internal(i16 8, i16 8, i8* %base, i64 %stride)
+  %d = call x86_amx @llvm.x86.tdpbssd.internal(i16 8, i16 8, i16 8, x86_amx %c, x86_amx %a, x86_amx %b)
+  call void @llvm.x86.tilestored64.internal(i16 8, i16 8, i8* %pointer, i64 %stride, x86_amx %d)
+
+  ret void
+}
+
+declare x86_amx @llvm.x86.tilezero.internal(i16, i16)
+declare x86_amx @llvm.x86.tileloadd64.internal(i16, i16, i8*, i64)
+declare x86_amx @llvm.x86.tdpbssd.internal(i16, i16, i16, x86_amx, x86_amx, x86_amx)
+declare void @llvm.x86.tilestored64.internal(i16, i16, i8*, i64, x86_amx)
Index: llvm/lib/Target/X86/X86RegisterInfo.td
===
--- llvm/lib/Target/X86/X86RegisterInfo.td
+++ llvm/lib/Target/X86/X86RegisterInfo.td
@@ -639,7 +639,7 @@
 let CopyCost = -1 in // Don't allow copying of tile registers
 def TILE : RegisterClass<"X86", [x86amx], 8192,
  (sequence "TMM%u", 0, 7)> {let Size = 8192;}
-def TILECFG : RegisterClass<"X86", [v512i1], 512, (add TMMCFG)> {
+def TILECFG : RegisterClass<"X86", [untyped], 512, (add TMMCFG)> {
   let CopyCost = -1;  // Don't allow copying of tile config registers.
   let isAllocatable = 1;
   let Size = 512;
Index: llvm/lib/Target/X86/X86RegisterInfo.cpp
===
--- llvm/lib/Target/X86/X86RegisterInfo.cpp
+++ llvm/lib/Target/X86/X86RegisterInfo.cpp
@@ -873,6 +873,7 @@
   // We only collect the tile shape that is defined.
   case X86::PTILELOADDV:
   case X86::PTDPBSSDV:
+  case X86::PTILEZEROV:
 MachineOperand &MO1 = MI->getOperand(1);
 MachineOperand &MO2 = MI->getOperand(2);
 ShapeT Shape(&MO1, &MO2, MRI);
Index: llvm/lib/Target/X86/X86PreTileConfig.cpp
===
--- llvm/lib/Target/X86/X86PreTileConfig.cpp
+++ llvm/lib/Target/X86/X86PreTileConfig.cpp
@@ -132,6 +132,7 @@
 llvm_unreachable("Unexpected machine instruction on tile");
   case X86::PTILELOADDV:
   case X86::PTDPBSSDV:
+  case X86::PTILEZEROV:
 MachineOperand &MO1 = const_cast(MI.getOperand(1));
 MachineOperand &MO2 = const_cast(MI.getOperand(2));
 ShapeT Shape(&MO1, &MO2, MRI);
@@ -230,6 +231,7 @@
   case X86::PTILELOADDV:
   case X86::PTILESTOREDV:
   case X86::PTDPBSSDV:
+  case X86::PTILEZEROV:
 unsigned NumOperands = MI.getNumOperands();
 MI.RemoveOperand(NumOperands - 1);
 MI.addOperand(MF, MachineOperand::CreateReg(CFG, false));
Index: llvm/lib/Target/X86/X86InstrAMX.td
===
--- llvm/lib/Target/X86/X86InstrAMX.td
+++ llvm/lib/Target/X86/X86InstrAMX.td
@@ -62,6 +62,9 @@
 def PTILESTOREDV : PseudoI<(outs), (ins GR16:$src1,
 GR16:$src2, opaquemem:$src3,
 TILE:$src4, 

[PATCH] D92782: [CodeGen][AMDGPU] Fix ICE for static initializer IR generation

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 310145.
bader added a comment.

Fix clang-format issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92782

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/test/CodeGen/address-space.c


Index: clang/test/CodeGen/address-space.c
===
--- clang/test/CodeGen/address-space.c
+++ clang/test/CodeGen/address-space.c
@@ -59,3 +59,14 @@
 void_ptr_arithmetic_test(void __attribute__((address_space(1))) *arg) {
 return arg + 4;
 }
+
+// CHECK-LABEL: define i32* @test5(
+const unsigned *test5() {
+  // Intentionally leave a part of an array uninitialized. This triggers a
+  // different code path contrary to a fully initialized array.
+  // CHECK: ret i32* getelementptr inbounds ([256 x i32]
+  static const unsigned bars[256] = {
+  0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10,
+  11, 12, 13, 14, 15, 16, 17, 18, 19, 20};
+  return &bars[0];
+}
Index: clang/lib/CodeGen/CGDecl.cpp
===
--- clang/lib/CodeGen/CGDecl.cpp
+++ clang/lib/CodeGen/CGDecl.cpp
@@ -353,12 +353,11 @@
   if (GV->getValueType() != Init->getType()) {
 llvm::GlobalVariable *OldGV = GV;
 
-GV = new llvm::GlobalVariable(CGM.getModule(), Init->getType(),
-  OldGV->isConstant(),
-  OldGV->getLinkage(), Init, "",
-  /*InsertBefore*/ OldGV,
-  OldGV->getThreadLocalMode(),
-   
CGM.getContext().getTargetAddressSpace(D.getType()));
+GV = new llvm::GlobalVariable(
+CGM.getModule(), Init->getType(), OldGV->isConstant(),
+OldGV->getLinkage(), Init, "",
+/*InsertBefore*/ OldGV, OldGV->getThreadLocalMode(),
+OldGV->getType()->getPointerAddressSpace());
 GV->setVisibility(OldGV->getVisibility());
 GV->setDSOLocal(OldGV->isDSOLocal());
 GV->setComdat(OldGV->getComdat());


Index: clang/test/CodeGen/address-space.c
===
--- clang/test/CodeGen/address-space.c
+++ clang/test/CodeGen/address-space.c
@@ -59,3 +59,14 @@
 void_ptr_arithmetic_test(void __attribute__((address_space(1))) *arg) {
 return arg + 4;
 }
+
+// CHECK-LABEL: define i32* @test5(
+const unsigned *test5() {
+  // Intentionally leave a part of an array uninitialized. This triggers a
+  // different code path contrary to a fully initialized array.
+  // CHECK: ret i32* getelementptr inbounds ([256 x i32]
+  static const unsigned bars[256] = {
+  0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10,
+  11, 12, 13, 14, 15, 16, 17, 18, 19, 20};
+  return &bars[0];
+}
Index: clang/lib/CodeGen/CGDecl.cpp
===
--- clang/lib/CodeGen/CGDecl.cpp
+++ clang/lib/CodeGen/CGDecl.cpp
@@ -353,12 +353,11 @@
   if (GV->getValueType() != Init->getType()) {
 llvm::GlobalVariable *OldGV = GV;
 
-GV = new llvm::GlobalVariable(CGM.getModule(), Init->getType(),
-  OldGV->isConstant(),
-  OldGV->getLinkage(), Init, "",
-  /*InsertBefore*/ OldGV,
-  OldGV->getThreadLocalMode(),
-   CGM.getContext().getTargetAddressSpace(D.getType()));
+GV = new llvm::GlobalVariable(
+CGM.getModule(), Init->getType(), OldGV->isConstant(),
+OldGV->getLinkage(), Init, "",
+/*InsertBefore*/ OldGV, OldGV->getThreadLocalMode(),
+OldGV->getType()->getPointerAddressSpace());
 GV->setVisibility(OldGV->getVisibility());
 GV->setDSOLocal(OldGV->isDSOLocal());
 GV->setComdat(OldGV->getComdat());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92627: Basic: Add hashing support for FileEntryRef and DirectoryEntryRef

[Jan Svoboda via Phabricator via cfe-commits]

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

Left a couple of nits and a question, but LGTM overall.




Comment at: clang/include/clang/Basic/DirectoryEntry.h:104
+  bool isSpecialDenseMapKey() const {
+return ME == llvm::DenseMapInfo::getEmptyKey() ||
+   ME == llvm::DenseMapInfo::getTombstoneKey();

Nit: would it make sense to avoid copy-pasting the constructor logic here 
(`llvm::DenseMapInfo::getEmptyKey()`) and call the 
constructor instead?
For example: `isSameRef(DirectoryEntryRef(dense_map_empty_tag{}))`.

The same goes for `FileEntryRef`.



Comment at: clang/include/clang/Basic/DirectoryEntry.h:212
+// Catch the easy cases: both empty, both tombstone, or the same ref.
+if (LHS.ME == RHS.ME)
+  return true;

Nit: could use `LHS.isSameRef(RHS)` to keep this uniform.



Comment at: clang/unittests/Basic/FileEntryTest.cpp:126
 
+TEST(FileEntryTest, DenseMapInfo) {
+  RefMaps Refs;

Question: do we prefer naming tests with the function/data structure under 
test, or using more descriptive names? (e.g. `DenseMapFindsOldestEntry`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92627

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

[PATCH] D92678: ARCMigrate: Migrate ObjCMT.cpp over to FileEntryRef

[Jan Svoboda via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92678

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

[PATCH] D92680: Frontend: Migrate to FileEntryRef in CompilerInstance::InitializeSourceManager, NFC

[Jan Svoboda via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92680

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

[PATCH] D92699: Frontend: Migrate to FileEntryRef in VerifyDiagnosticConsumer.cpp, NFC

[Jan Svoboda via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92699

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

[PATCH] D92837: [X86] Support tilezero intrinsic and c interface for AMX.

[Pengfei Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: llvm/lib/Target/X86/X86ExpandPseudo.cpp:499
+MI.RemoveOperand(3); // Remove $tmmcfg
+for (int i = 2; i > 0; --i)
+  MI.RemoveOperand(i);

Can change to i = 3?



Comment at: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:4624
+  SDValue Chain = Node->getOperand(0);
+  SDValue Ops[] = {Node->getOperand(2), Node->getOperand(3), CFG, Chain};
+  MachineSDNode *CNode =

Why tilezero needs chain?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92837

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

[PATCH] D92837: [X86] Support tilezero intrinsic and c interface for AMX.

[LuoYuanke via Phabricator via cfe-commits]

LuoYuanke added inline comments.



Comment at: llvm/lib/Target/X86/X86ExpandPseudo.cpp:499
+MI.RemoveOperand(3); // Remove $tmmcfg
+for (int i = 2; i > 0; --i)
+  MI.RemoveOperand(i);

pengfei wrote:
> Can change to i = 3?
Yes, then remove line 498.



Comment at: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:4624
+  SDValue Chain = Node->getOperand(0);
+  SDValue Ops[] = {Node->getOperand(2), Node->getOperand(3), CFG, Chain};
+  MachineSDNode *CNode =

pengfei wrote:
> Why tilezero needs chain?
I didn't declare "IntrNoMem" in "include/llvm/IR/IntrinsicsX86.td". I can 
revise the code to declare "IntrNoMem".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92837

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

[PATCH] D90275: [clang][IR] Add support for leaf attribute

[Aaron Ballman via Phabricator via cfe-commits]

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

Aside from a testing request, the frontend parts LGTM. I don't know enough 
about the LLVM side to feel comfortable signing off on it. @jdoerfert, would 
you mind approving that part?




Comment at: clang/test/Sema/attr-leaf.c:4
+
+void f() __attribute__((leaf));

Can you also add tests that show the attribute diagnoses being written on the 
wrong subject or when given arguments? Also, can you add a test showing the 
attribute on a function definition with a FIXME comment about wanting to 
diagnose that case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90275

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

[PATCH] D92600: [ASTImporter] Add support for importing GenericSelectionExpr AST nodes.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2366
+/// Matches C11 _Generic expression.
+extern const internal::VariadicDynCastAllOfMatcher
+genericSelectionExpr;

tmroeder wrote:
> aaron.ballman wrote:
> > Do we have a use case for adding this as an AST matcher? For instance, one 
> > of the first things I'd want to do with such a matcher is traverse down to 
> > the association list, but I don't think that's possible currently. Should 
> > we add all of the AST functionality or just wait until there's a concrete 
> > use case and add the AST matchers bits at that point?
> I mostly did this because that's what I did in the previous case from Feb 
> 2019: reviews.llvm.org/D58292, for ChooseExpr, and I was cribbing off my old 
> code.
> 
> However, on further reflection, I remembered that this patch actually removes 
> a case that was locally defining this matcher already: see the removed lines 
> in clang/lib/Analysis/ExprMutationAnalyzer.cpp in this patch. So, I think 
> that is enough of a case for adding this.
Okay, fair enough. Thanks!



Comment at: clang/test/ASTMerge/generic-selection-expr/Inputs/generic.c:1
+void f() {
+  int x;

Should we also have a C++ test for a result-dependent use of _Generic? (We 
support use of _Generic in C++ as an extension.)



Comment at: clang/unittests/AST/StructuralEquivalenceTest.cpp:1601
 
+TEST_F(StructuralEquivalenceStmtTest, GenericSelectionExprSame) {
+  auto t = makeWrappedStmts("_Generic(0u, unsigned int: 0, float: 1)",

Should we add a structural equivalence test for a dependent result type in C++?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92600

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

[PATCH] D92155: Load plugins when creating a CompilerInvocation.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D92155#2419667 , @psionic12 wrote:

>> - main file AST build: Important to run plugins here but it's critical they 
>> don't traverse the whole preamble (can degrade latency by orders of 
>> magnitude).
>
> Well, Sound reasonable, but what if `getTranslationUnitDecl()` is indeed 
> needed instead of using `TraverseAST` (I didn't came up with an example yet)? 
> In this case I think clangd should allow a plugin to do its job, even if it 
> is performance hostile.

To a first approximation, I think we *should* disable features that don't 
perform well, as the overall responsiveness is more important than any one 
feature.
We make numerous functionality tradeoffs in clangd consistent with this.

As a practical concern, I'm not sure what we can reasonably do to enforce this 
though.

(One idea we've been playing with is that traversal of the AST usually goes 
through TranslationUnitDecls::decls_begin(), which triggers loading from the 
preamble. We could add a stateful flag to DeclContext that causes this to 
behave like noload_decls_begin instead...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92155

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

[PATCH] D91925: [clangd][NFC] Small tweak to combined provider

[Sam McCall via Phabricator via cfe-commits]

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

(I'm not sure how it resolves the problem, but certainly looks nicer!)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91925

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

[PATCH] D92825: [clang][cli] CompilerInvocationTest: join two test fixtures into one

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92825

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

[PATCH] D92826: [clang][cli] CompilerInvocationTest: rename member variable in fixture

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92826

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

[PATCH] D92827: [clang][cli] CompilerInvocationTest: split enum test into two

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92827

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

[PATCH] D92828: [clang][cli] CompilerInvocationTest: remove unnecessary command line arguments

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92828

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

[PATCH] D92829: [clang][cli] CompilerInvocationTest: check arg parsing does not produce diagnostics

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92829

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

[PATCH] D92830: [clang][cli] CompilerInvocationTest: join and add test cases

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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

LGTM; thanks for splitting out the prep commits!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92830

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

[PATCH] D92774: [clang][cli] CompilerInvocationTest: add tests for boolean options

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

WIP, need to fix a typo in the test.




Comment at: clang/unittests/Frontend/CompilerInvocationTest.cpp:141
+  ASSERT_TRUE(Diags->hasErrorOccurred()); // Driver-only flag.
+  ASSERT_TRUE(Invocation.getCodeGenOpts().Autolink);
+}

This is wrong, let me fix that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92774

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

[PATCH] D92774: [clang][cli] CompilerInvocationTest: add tests for boolean options

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 310176.
jansvoboda11 added a comment.

Fix typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92774

Files:
  clang/unittests/Frontend/CompilerInvocationTest.cpp

Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -77,6 +77,150 @@
   ASSERT_TRUE(Invocation.getFrontendOpts().UseTemporary);
 }
 
+// Boolean option with a keypath that defaults to true.
+// The flag with negative spelling can set the keypath to false.
+// The flag with positive spelling can reset the keypath to true.
+
+TEST_F(CommandLineTest, BoolOptionDefaultTruePresentNone) {
+  const char *Args[] = {""};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_TRUE(Invocation.getCodeGenOpts().Autolink);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultTruePresentNegChange) {
+  const char *Args[] = {"-fno-autolink"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_FALSE(Invocation.getCodeGenOpts().Autolink);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultTruePresentPosReset) {
+  const char *Args[] = {"-fautolink"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_TRUE(Diags->hasErrorOccurred()); // Driver-only flag.
+  ASSERT_TRUE(Invocation.getCodeGenOpts().Autolink);
+}
+
+// Boolean option with a keypath that defaults to false.
+// The flag with negative spelling can set the keypath to true.
+// The flag with positive spelling can reset the keypath to false.
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentNone) {
+  const char *Args[] = {""};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_FALSE(Invocation.getCodeGenOpts().NoInlineLineTables);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentNegChange) {
+  const char *Args[] = {"-gno-inline-line-tables"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_TRUE(Invocation.getCodeGenOpts().NoInlineLineTables);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentPosReset) {
+  const char *Args[] = {"-ginline-line-tables"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_TRUE(Diags->hasErrorOccurred()); // Driver-only flag.
+  ASSERT_FALSE(Invocation.getCodeGenOpts().NoInlineLineTables);
+}
+
+// Boolean option with a keypath that defaults to false.
+// The flag with positive spelling can set the keypath to true.
+// The flag with negative spelling can reset the keypath to false.
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentNoneX) {
+  const char *Args[] = {""};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_FALSE(Invocation.getCodeGenOpts().CodeViewGHash);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentPosChange) {
+  const char *Args[] = {"-gcodeview-ghash"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_TRUE(Invocation.getCodeGenOpts().CodeViewGHash);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentNegReset) {
+  const char *Args[] = {"-gno-codeview-ghash"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_TRUE(Diags->hasErrorOccurred()); // Driver-only flag.
+  ASSERT_FALSE(Invocation.getCodeGenOpts().CodeViewGHash);
+}
+
+// Boolean option with a keypath that defaults to an arbitrary expression.
+// The flag with positive spelling can set the keypath to true.
+// The flag with negative spelling can set the keypath to false.
+
+// NOTE: The following tests need to be updated when we start enabling the new
+// pass manager by default.
+
+TEST_F(CommandLineTest, BoolOptionDefaultArbitraryTwoFlagsPresentNone) {
+  const char *Args = {""};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_FALSE(Invocation.getCodeGenOpts().ExperimentalNewPassManager);
+
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+  ASSERT_THAT(GeneratedArgs,
+  Not(Contains(StrEq("-fexperimental-new-pass-manager";
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultArbitraryTwoFlagsPresentPos) {
+  const char *Args[] = {"-fexperimental-new-pass-manager"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *

[PATCH] D91025: [clangd] Fix locateMacroAt() for macro definition outside preamble

[Sam McCall via Phabricator via cfe-commits]

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

Nice catch! As always, sorry about the delay.




Comment at: clang-tools-extra/clangd/SourceCode.cpp:987
+  FileID FID = SM.getFileID(Loc);
+  auto JustAfterToken =
+  SM.getLocForEndOfFile(FID) == Loc ? Loc : Loc.getLocWithOffset(1);

nit: I guess these would be clearer with auto -> SourceLocation



Comment at: clang-tools-extra/clangd/SourceCode.cpp:988
+  auto JustAfterToken =
+  SM.getLocForEndOfFile(FID) == Loc ? Loc : Loc.getLocWithOffset(1);
+  auto *MI =

I'm a little confused about the condition here: Loc points at the macro name, 
which must be at least 1 character long, so we can't be at EOF, right?

Unless i'm missing something, I think we can promote to an assert



Comment at: clang-tools-extra/clangd/SourceCode.cpp:993
+auto JustBeforeToken =
+SM.getLocForStartOfFile(FID) == Loc ? Loc : Loc.getLocWithOffset(-1);
+MI = PP.getMacroDefinitionAtLoc(IdentifierInfo, JustBeforeToken)

on the other hand, I suppose this case *is* possible with e.g. a builtin macro 
used at the start of the file.

I think hoisting this into the condition might be clearer by avoiding the 
redundant case (the actual *performance* doesn't matter, just for readability)
```
// If this is foo in `#undef foo`, use the old definition.
if (!MI && SM.getLocForStartOfFile(FID) != Loc)
  MI = PP.getMacroDefinitionAtLoc(II, Loc.getLocWithOffset(-1)).getMacroInfo();
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91025

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

[PATCH] D92774: [clang][cli] CompilerInvocationTest: add tests for boolean options

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

Ready to be reviewed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92774

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

[PATCH] D92775: [clang][cli] Add flexible TableGen multiclass for boolean options

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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

LGTM if you undo the change to lose support for AlwaysEmit (happy to consider 
in a separate patch if it’s the right thing to do though).




Comment at: clang/include/clang/Driver/Options.td:371
+  // TODO: Assert that the flags have different value.
+  // TODO: Assert that only one of the flags can be implied.
+

jansvoboda11 wrote:
> Does TableGen support some kind of assertion mechanism?
Not that I’m aware of. @Bigcheese?



Comment at: clang/unittests/Frontend/CompilerInvocationTest.cpp:226
   ASSERT_THAT(GeneratedArgs,
-  Contains(StrEq("-fno-experimental-new-pass-manager")));
+  Not(Contains(StrEq("-fno-experimental-new-pass-manager";
   ASSERT_THAT(GeneratedArgs,

jansvoboda11 wrote:
> dexonsmith wrote:
> > Can you clarify why this was dropped? Was it previously emitted due to a 
> > limitation in the implementation, or are we no longer supporting options 
> > that always get emitted for clarity?
> This option was the only one using the old infrastructure 
> (`BooleanMarshalledFFlag`).
> It was set up to always generate the flag, even when it wasn't necessary 
> (this flag sets the keypath to its default value).
> 
> I think we should aim to generate only command line arguments that are 
> necessary to preserve the original invocation semantics.
> I imagine this will be useful when debugging: one won't need to scan hundreds 
> of flags that don't have any effect on CompilerInvocation.
This is a change in direction; the original thinking was that some options 
should always be emitted for human readability. I don’t feel too strongly about 
it, but I think this should be changed / dropped independently of other work if 
it’s done. I suggest splitting this out; I’d also like to hear @Bigcheese’s 
thoughts on that change since he did more of the original reviews. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92775

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

[PATCH] D92774: [clang][cli] CompilerInvocationTest: add tests for boolean options

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92774

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

[clang-tools-extra] f6b205d - [clangd] ExtractFunction: disable on regions that sometimes, but not always return.

[Adam Czachorowski via cfe-commits]

Author: Adam Czachorowski
Date: 2020-12-08T15:55:32+01:00
New Revision: f6b205dae16392382324fbca676ef6afe3920642

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

LOG: [clangd] ExtractFunction: disable on regions that sometimes, but not 
always return.

apply() will fail in those cases, so it's better to detect it in
prepare() already and hide code action from the user.

This was especially annoying on code bases that use a lot of
RETURN_IF_ERROR-like macros.

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

Added: 


Modified: 
clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
clang-tools-extra/clangd/unittests/TweakTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp 
b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
index 18fe7bf39160..8eec42876d6b 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -708,6 +708,27 @@ tooling::Replacement createFunctionDefinition(const 
NewFunction &ExtractedFunc,
   return tooling::Replacement(SM, ExtractedFunc.InsertionPoint, 0, 
FunctionDef);
 }
 
+// Returns true if ExtZone contains any ReturnStmts.
+bool hasReturnStmt(const ExtractionZone &ExtZone) {
+  class ReturnStmtVisitor
+  : public clang::RecursiveASTVisitor {
+  public:
+bool VisitReturnStmt(ReturnStmt *Return) {
+  Found = true;
+  return false; // We found the answer, abort the scan.
+}
+bool Found = false;
+  };
+
+  ReturnStmtVisitor V;
+  for (const Stmt *RootStmt : ExtZone.RootStmts) {
+V.TraverseStmt(const_cast(RootStmt));
+if (V.Found)
+  break;
+  }
+  return V.Found;
+}
+
 bool ExtractFunction::prepare(const Selection &Inputs) {
   const LangOptions &LangOpts = Inputs.AST->getLangOpts();
   if (!LangOpts.CPlusPlus)
@@ -715,8 +736,12 @@ bool ExtractFunction::prepare(const Selection &Inputs) {
   const Node *CommonAnc = Inputs.ASTSelection.commonAncestor();
   const SourceManager &SM = Inputs.AST->getSourceManager();
   auto MaybeExtZone = findExtractionZone(CommonAnc, SM, LangOpts);
+  if (!MaybeExtZone ||
+  (hasReturnStmt(*MaybeExtZone) && !alwaysReturns(*MaybeExtZone)))
+return false;
+
   // FIXME: Get rid of this check once we support hoisting.
-  if (!MaybeExtZone || MaybeExtZone->requiresHoisting(SM))
+  if (MaybeExtZone->requiresHoisting(SM))
 return false;
 
   ExtZone = std::move(*MaybeExtZone);

diff  --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp 
b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index 07f061b3f2e3..85edd92d27da 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -607,7 +607,11 @@ TEST_F(ExtractFunctionTest, FunctionTest) {
   // Extract certain return
   EXPECT_THAT(apply(" if(true) [[{ return; }]] "), HasSubstr("extracted"));
   // Don't extract uncertain return
-  EXPECT_THAT(apply(" if(true) [[if (false) return;]] "), StartsWith("fail"));
+  EXPECT_THAT(apply(" if(true) [[if (false) return;]] "),
+  StartsWith("unavailable"));
+  EXPECT_THAT(
+  apply("#define RETURN_IF_ERROR(x) if (x) return\nRETU^RN_IF_ERROR(4);"),
+  StartsWith("unavailable"));
 
   FileName = "a.c";
   EXPECT_THAT(apply(" for([[int i = 0;]];);"), HasSubstr("unavailable"));



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

[PATCH] D92408: [clangd] ExtractFunction: disable on regions that sometimes, but not always return.

[Adam Czachorowski 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 rGf6b205dae163: [clangd] ExtractFunction: disable on regions 
that sometimes, but not always… (authored by adamcz).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92408

Files:
  clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -607,7 +607,11 @@
   // Extract certain return
   EXPECT_THAT(apply(" if(true) [[{ return; }]] "), HasSubstr("extracted"));
   // Don't extract uncertain return
-  EXPECT_THAT(apply(" if(true) [[if (false) return;]] "), StartsWith("fail"));
+  EXPECT_THAT(apply(" if(true) [[if (false) return;]] "),
+  StartsWith("unavailable"));
+  EXPECT_THAT(
+  apply("#define RETURN_IF_ERROR(x) if (x) return\nRETU^RN_IF_ERROR(4);"),
+  StartsWith("unavailable"));
 
   FileName = "a.c";
   EXPECT_THAT(apply(" for([[int i = 0;]];);"), HasSubstr("unavailable"));
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -708,6 +708,27 @@
   return tooling::Replacement(SM, ExtractedFunc.InsertionPoint, 0, 
FunctionDef);
 }
 
+// Returns true if ExtZone contains any ReturnStmts.
+bool hasReturnStmt(const ExtractionZone &ExtZone) {
+  class ReturnStmtVisitor
+  : public clang::RecursiveASTVisitor {
+  public:
+bool VisitReturnStmt(ReturnStmt *Return) {
+  Found = true;
+  return false; // We found the answer, abort the scan.
+}
+bool Found = false;
+  };
+
+  ReturnStmtVisitor V;
+  for (const Stmt *RootStmt : ExtZone.RootStmts) {
+V.TraverseStmt(const_cast(RootStmt));
+if (V.Found)
+  break;
+  }
+  return V.Found;
+}
+
 bool ExtractFunction::prepare(const Selection &Inputs) {
   const LangOptions &LangOpts = Inputs.AST->getLangOpts();
   if (!LangOpts.CPlusPlus)
@@ -715,8 +736,12 @@
   const Node *CommonAnc = Inputs.ASTSelection.commonAncestor();
   const SourceManager &SM = Inputs.AST->getSourceManager();
   auto MaybeExtZone = findExtractionZone(CommonAnc, SM, LangOpts);
+  if (!MaybeExtZone ||
+  (hasReturnStmt(*MaybeExtZone) && !alwaysReturns(*MaybeExtZone)))
+return false;
+
   // FIXME: Get rid of this check once we support hoisting.
-  if (!MaybeExtZone || MaybeExtZone->requiresHoisting(SM))
+  if (MaybeExtZone->requiresHoisting(SM))
 return false;
 
   ExtZone = std::move(*MaybeExtZone);


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -607,7 +607,11 @@
   // Extract certain return
   EXPECT_THAT(apply(" if(true) [[{ return; }]] "), HasSubstr("extracted"));
   // Don't extract uncertain return
-  EXPECT_THAT(apply(" if(true) [[if (false) return;]] "), StartsWith("fail"));
+  EXPECT_THAT(apply(" if(true) [[if (false) return;]] "),
+  StartsWith("unavailable"));
+  EXPECT_THAT(
+  apply("#define RETURN_IF_ERROR(x) if (x) return\nRETU^RN_IF_ERROR(4);"),
+  StartsWith("unavailable"));
 
   FileName = "a.c";
   EXPECT_THAT(apply(" for([[int i = 0;]];);"), HasSubstr("unavailable"));
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -708,6 +708,27 @@
   return tooling::Replacement(SM, ExtractedFunc.InsertionPoint, 0, FunctionDef);
 }
 
+// Returns true if ExtZone contains any ReturnStmts.
+bool hasReturnStmt(const ExtractionZone &ExtZone) {
+  class ReturnStmtVisitor
+  : public clang::RecursiveASTVisitor {
+  public:
+bool VisitReturnStmt(ReturnStmt *Return) {
+  Found = true;
+  return false; // We found the answer, abort the scan.
+}
+bool Found = false;
+  };
+
+  ReturnStmtVisitor V;
+  for (const Stmt *RootStmt : ExtZone.RootStmts) {
+V.TraverseStmt(const_cast(RootStmt));
+if (V.Found)
+  break;
+  }
+  return V.Found;
+}
+
 bool ExtractFunction::prepare(const Selection &Inputs) {
   const LangOptions &LangOpts = Inputs.AST->getLangOpts();
   if (!LangOpts.CPlusPlus)
@@ -715,8 +736,12 @@
   const Node *CommonAnc = Inputs.ASTSelection.commonAncestor();
   const SourceManager &SM = Inputs.AST->getSourceManager();
   auto MaybeExtZone = findExtr

[PATCH] D92810: [clang-tidy] Recognize single character needles for absl::StrContains.

[Chris Kennelly 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 rG16622d535c02: [clang-tidy] Recognize single character 
needles for absl::StrContains. (authored by ckennelly).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92810

Files:
  clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp
@@ -226,17 +226,21 @@
   // CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(asv, cc);{{$}}
 }
 
-// Confirms that it does *not* match when the parameter to find() is a char,
-// because absl::StrContains is not implemented for char.
-void no_char_param_tests() {
+void char_param_tests() {
   std::string ss;
   ss.find('c') == std::string::npos;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
+  // CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(ss, 'c');{{$}}
 
   std::string_view ssv;
   ssv.find('c') == std::string_view::npos;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
+  // CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(ssv, 'c');{{$}}
 
   absl::string_view asv;
   asv.find('c') == absl::string_view::npos;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
+  // CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(asv, 'c');{{$}}
 }
 
 #define FOO(a, b, c, d) ((a).find(b) == std::string::npos ? (c) : (d))
Index: clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp
===
--- clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp
+++ clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp
@@ -31,6 +31,8 @@
 using ::clang::transformer::node;
 using ::clang::transformer::RewriteRule;
 
+AST_MATCHER(Type, isCharType) { return Node.isCharType(); }
+
 static const char DefaultStringLikeClasses[] = "::std::basic_string;"
"::std::basic_string_view;"
"::absl::string_view";
@@ -58,13 +60,15 @@
   hasUnqualifiedDesugaredType(recordType(hasDeclaration(StringLikeClass)));
   auto CharStarType =
   hasUnqualifiedDesugaredType(pointerType(pointee(isAnyCharacter(;
+  auto CharType = hasUnqualifiedDesugaredType(isCharType());
   auto StringNpos = declRefExpr(
   to(varDecl(hasName("npos"), hasDeclContext(StringLikeClass;
   auto StringFind = cxxMemberCallExpr(
   callee(cxxMethodDecl(
   hasName("find"),
-  hasParameter(0, parmVarDecl(anyOf(hasType(StringType),
-hasType(CharStarType)),
+  hasParameter(
+  0, parmVarDecl(anyOf(hasType(StringType), hasType(CharStarType),
+   hasType(CharType)),
   on(hasType(StringType)), hasArgument(0, 
expr().bind("parameter_to_find")),
   anyOf(hasArgument(1, integerLiteral(equals(0))),
 hasArgument(1, cxxDefaultArgExpr())),


Index: clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp
@@ -226,17 +226,21 @@
   // CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(asv, cc);{{$}}
 }
 
-// Confirms that it does *not* match when the parameter to find() is a char,
-// because absl::StrContains is not implemented for char.
-void no_char_param_tests() {
+void char_param_tests() {
   std::string ss;
   ss.find('c') == std::string::npos;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
+  // CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(ss, 'c');{{$}}
 
   std::string_view ssv;
   ssv.find('c') == std::string_view::npos;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
+  // CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(ssv, 'c');{{$}}
 
   absl::string_view asv;
   asv.find('c') == absl::string_view::npos;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
+  // CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(asv, 'c');{{$}}
 }
 
 #define FOO(a, b, c, d) ((a).find(b) == std::string::npos ? (c) : (d))
Index: clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp
==

[clang-tools-extra] 16622d5 - [clang-tidy] Recognize single character needles for absl::StrContains.

[Chris Kennelly via cfe-commits]

Author: Chris Kennelly
Date: 2020-12-08T10:01:30-05:00
New Revision: 16622d535c021b566c1304b6388a6399e606

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

LOG: [clang-tidy] Recognize single character needles for absl::StrContains.

Commit fbdff6f3ae0b in the Abseil tree adds an overload for
absl::StrContains to accept a single character needle for optimized
lookups.

Reviewed By: hokein

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp

clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp 
b/clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp
index cd890e4837e0..977c1919cee3 100644
--- a/clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp
@@ -31,6 +31,8 @@ using ::clang::transformer::makeRule;
 using ::clang::transformer::node;
 using ::clang::transformer::RewriteRule;
 
+AST_MATCHER(Type, isCharType) { return Node.isCharType(); }
+
 static const char DefaultStringLikeClasses[] = "::std::basic_string;"
"::std::basic_string_view;"
"::absl::string_view";
@@ -58,13 +60,15 @@ MakeRule(const LangOptions &LangOpts,
   hasUnqualifiedDesugaredType(recordType(hasDeclaration(StringLikeClass)));
   auto CharStarType =
   hasUnqualifiedDesugaredType(pointerType(pointee(isAnyCharacter(;
+  auto CharType = hasUnqualifiedDesugaredType(isCharType());
   auto StringNpos = declRefExpr(
   to(varDecl(hasName("npos"), hasDeclContext(StringLikeClass;
   auto StringFind = cxxMemberCallExpr(
   callee(cxxMethodDecl(
   hasName("find"),
-  hasParameter(0, parmVarDecl(anyOf(hasType(StringType),
-hasType(CharStarType)),
+  hasParameter(
+  0, parmVarDecl(anyOf(hasType(StringType), hasType(CharStarType),
+   hasType(CharType)),
   on(hasType(StringType)), hasArgument(0, 
expr().bind("parameter_to_find")),
   anyOf(hasArgument(1, integerLiteral(equals(0))),
 hasArgument(1, cxxDefaultArgExpr())),

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp
index 9d4b03aa22f2..81a3fc4d3b97 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp
@@ -226,17 +226,21 @@ void string_literal_and_char_ptr_tests() {
   // CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(asv, cc);{{$}}
 }
 
-// Confirms that it does *not* match when the parameter to find() is a char,
-// because absl::StrContains is not implemented for char.
-void no_char_param_tests() {
+void char_param_tests() {
   std::string ss;
   ss.find('c') == std::string::npos;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
+  // CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(ss, 'c');{{$}}
 
   std::string_view ssv;
   ssv.find('c') == std::string_view::npos;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
+  // CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(ssv, 'c');{{$}}
 
   absl::string_view asv;
   asv.find('c') == absl::string_view::npos;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
+  // CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(asv, 'c');{{$}}
 }
 
 #define FOO(a, b, c, d) ((a).find(b) == std::string::npos ? (c) : (d))



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

[PATCH] D92762: [clang][AArch64][SVE] Avoid going through memory for coerced VLST arguments

[Joe Ellis via Phabricator via cfe-commits]

joechrisellis updated this revision to Diff 310185.
joechrisellis added a comment.

Do not try to generate `llvm.experimental.vector.extract` instructions if the 
vector element types differ.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92762

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS.c
  clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS.cpp
  clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c

Index: clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c
===
--- clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c
+++ clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c
@@ -13,11 +13,8 @@
 
 // CHECK-LABEL: @to_svint32_t(
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:[[TYPE:%.*]] = alloca <16 x i32>, align 16
-// CHECK-NEXT:[[TMP0:%.*]] = bitcast <16 x i32>* [[TYPE]] to *
-// CHECK-NEXT:store  [[TYPE_COERCE:%.*]], * [[TMP0]], align 16
-// CHECK-NEXT:[[TYPE1:%.*]] = load <16 x i32>, <16 x i32>* [[TYPE]], align 16, [[TBAA6:!tbaa !.*]]
-// CHECK-NEXT:[[CASTSCALABLESVE:%.*]] = call  @llvm.experimental.vector.insert.nxv4i32.v16i32( undef, <16 x i32> [[TYPE1]], i64 0)
+// CHECK-NEXT:[[TYPE:%.*]] = call <16 x i32> @llvm.experimental.vector.extract.v16i32.nxv4i32( [[X_COERCE:%.*]], i64 0)
+// CHECK-NEXT:[[CASTSCALABLESVE:%.*]] = call  @llvm.experimental.vector.insert.nxv4i32.v16i32( undef, <16 x i32> [[TYPE]], i64 0)
 // CHECK-NEXT:ret  [[CASTSCALABLESVE]]
 //
 svint32_t to_svint32_t(fixed_int32_t type) {
@@ -39,11 +36,8 @@
 
 // CHECK-LABEL: @to_svfloat64_t(
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:[[TYPE:%.*]] = alloca <8 x double>, align 16
-// CHECK-NEXT:[[TMP0:%.*]] = bitcast <8 x double>* [[TYPE]] to *
-// CHECK-NEXT:store  [[TYPE_COERCE:%.*]], * [[TMP0]], align 16
-// CHECK-NEXT:[[TYPE1:%.*]] = load <8 x double>, <8 x double>* [[TYPE]], align 16, [[TBAA6]]
-// CHECK-NEXT:[[CASTSCALABLESVE:%.*]] = call  @llvm.experimental.vector.insert.nxv2f64.v8f64( undef, <8 x double> [[TYPE1]], i64 0)
+// CHECK-NEXT:[[TYPE:%.*]] = call <8 x double> @llvm.experimental.vector.extract.v8f64.nxv2f64( [[X_COERCE:%.*]], i64 0)
+// CHECK-NEXT:[[CASTSCALABLESVE:%.*]] = call  @llvm.experimental.vector.insert.nxv2f64.v8f64( undef, <8 x double> [[TYPE]], i64 0)
 // CHECK-NEXT:ret  [[CASTSCALABLESVE]]
 //
 svfloat64_t to_svfloat64_t(fixed_float64_t type) {
@@ -69,7 +63,7 @@
 // CHECK-NEXT:[[TYPE_ADDR:%.*]] = alloca <8 x i8>, align 16
 // CHECK-NEXT:[[TMP0:%.*]] = bitcast <8 x i8>* [[TYPE]] to *
 // CHECK-NEXT:store  [[TYPE_COERCE:%.*]], * [[TMP0]], align 16
-// CHECK-NEXT:[[TYPE1:%.*]] = load <8 x i8>, <8 x i8>* [[TYPE]], align 16, [[TBAA6]]
+// CHECK-NEXT:[[TYPE1:%.*]] = load <8 x i8>, <8 x i8>* [[TYPE]], align 16, [[TBAA6:!tbaa !.*]]
 // CHECK-NEXT:store <8 x i8> [[TYPE1]], <8 x i8>* [[TYPE_ADDR]], align 16, [[TBAA6]]
 // CHECK-NEXT:[[TMP1:%.*]] = bitcast <8 x i8>* [[TYPE_ADDR]] to *
 // CHECK-NEXT:[[TMP2:%.*]] = load , * [[TMP1]], align 16, [[TBAA6]]
@@ -130,11 +124,8 @@
 
 // CHECK-LABEL: @from_fixed_int32_t__to_gnu_int32_t(
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:[[TYPE:%.*]] = alloca <16 x i32>, align 16
-// CHECK-NEXT:[[TMP0:%.*]] = bitcast <16 x i32>* [[TYPE]] to *
-// CHECK-NEXT:store  [[TYPE_COERCE:%.*]], * [[TMP0]], align 16
-// CHECK-NEXT:[[TYPE1:%.*]] = load <16 x i32>, <16 x i32>* [[TYPE]], align 16, [[TBAA6]]
-// CHECK-NEXT:store <16 x i32> [[TYPE1]], <16 x i32>* [[AGG_RESULT:%.*]], align 16, [[TBAA6]]
+// CHECK-NEXT:[[TYPE:%.*]] = call <16 x i32> @llvm.experimental.vector.extract.v16i32.nxv4i32( [[X_COERCE:%.*]], i64 0)
+// CHECK-NEXT:store <16 x i32> [[TYPE]], <16 x i32>* [[AGG_RESULT:%.*]], align 16, [[TBAA6]]
 // CHECK-NEXT:ret void
 //
 gnu_int32_t from_fixed_int32_t__to_gnu_int32_t(fixed_int32_t type) {
Index: clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
===
--- clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
+++ clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
@@ -24,17 +24,14 @@
 
 // CHECK-LABEL: @fixed_caller(
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:[[X:%.*]] = alloca <16 x i32>, align 16
 // CHECK-NEXT:[[RETVAL_COERCE:%.*]] = alloca , align 16
-// CHECK-NEXT:[[TMP0:%.*]] = bitcast <16 x i32>* [[X]] to *
-// CHECK-NEXT:store  [[X_COERCE:%.*]], * [[TMP0]], align 16
-// CHECK-NEXT:[[X1:%.*]] = load <16 x i32>, <16 x i32>* [[X]], align 16, [[TBAA6:!tbaa !.*]]
-// CHECK-NEXT:[[CASTSCALABLESVE:%.*]] = call  @llvm.experimental.vector.insert.nxv4i32.v16i32( undef, <16 x i32> [[X1]], i64 0)
+// CHECK-NEXT:[[X:%.*]] = call <16 x i32> @llvm.experimental.vector.extract.v16i32.nxv4i32( [[X_COERCE:%.*]], i64 0)
+// CHECK-NE

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D89909#2423750 , @bader wrote:

>> It was mentioned that the changes in the type system with address spaces is 
>> undesirable for SYCL because you indend to parse existing C++ code as-is. 
>> This contradicts the intended semantic of address spaces where the whole 
>> point of it is to modify the standard types and therefore a compilation of 
>> C++ with the standard semantic is only guaranteed when the attribute is not 
>> used at all.
>
> Right, but I don't think it's related to the address space attributes. It was 
> mentioned in the context of re-using OpenCL *mode* for SYCL device 
> compilation, which modifies types which does use address space attribute 
> explicitly. "Existing C++ code" doesn't use address space attributes and our 
> solution does differentiate explicitly annotated type. The difference with 
> OpenCL mode is that SYCL doesn't change types by modifying "default" address 
> space attribute and allows conversion from/to "default" address space. As I 
> mentioned in RFC, according to my understanding this patch doesn't contradict 
> Embedded C requirements regarding address space attribute usage. I think the 
> spec allows an implementation to define conversion rules between address 
> spaces and doesn't imply type change based on the declaration scope - it's 
> OpenCL specific behavior.

Ok, if you plan to follow the Embedded C semantic (which your current patch 
confirms) then you should just reuse the existing target address spaces and 
extend the implementation with the ability to specify the relation among 
address spaces. The patch that you have mentioned earlier 
https://reviews.llvm.org/D62574 is providing this logic already and it looks 
good aside from testing. I would suggest to discuss with @ebevhan the timeline 
for committing it as the testing could be done using SYCL implementation. 
Alternatively, you could consider reusing the relevant parts of the patch if 
@ebevhan has no objections to that.

>> Perhaps to unblock your work it would be good to have a summary of what 
>> functionality you require from the address space attribute and what needs to 
>> work differently. Then at least it would be more clear if the attribute is a 
>> good fit here and if so what difference in its semantic will be brought by 
>> SYCL customizations. Whichever route we decide to go ahead, the 
>> documentation of intended language semantic should be added somewhere 
>> publicly accessible to facilitate adequate code review and maintenance 
>> because as far as I am aware it is not documented as part of the SYCL spec 
>> or any other documentation resource.
>
> The only difference directly related to clang's "opencl_*" address space 
> attributes is that SYCL allows conversion between types with OpenCL address 
> space attributes and "default" address space, but it's an implementation 
> detail. The main sematic difference is that SYCL doesn't change "default" 
> address space and it's "visible" with template metaprogramming or type 
> deduction at compile time. It looks like the best place to describe this 
> difference is a separate document similar to 
> https://clang.llvm.org/docs/OpenCLSupport.html, right?

Yes, this should be good enough. I imagine you won't need a lot to document if 
you reuse target address space and an extension for targets to configure 
address space relations mentioned above (which we should document in the common 
C/C++ extensions btw).




Comment at: clang/lib/Basic/Targets.cpp:577
-  case llvm::Triple::spir: {
-if (Triple.getOS() != llvm::Triple::UnknownOS ||
-Triple.getEnvironment() != llvm::Triple::UnknownEnvironment)

We should not allow any arbitrary OS/Environment for SPIR.



Comment at: clang/lib/Basic/Targets/SPIR.h:71
 LongWidth = LongAlign = 64;
-AddrSpaceMap = &SPIRAddrSpaceMap;
+if (Triple.getEnvironment() == llvm::Triple::SYCLDevice) {
+  AddrSpaceMap = &SYCLAddrSpaceMap;

This deserves clarification - address space map reflect the physical address 
space of the target - how will upstream users get to physical address spaces of 
targets for SYCL?



Comment at: clang/lib/CodeGen/TargetInfo.cpp:10019
+
+LangAS SPIRTargetCodeGenInfo::getGlobalVarAddressSpace(CodeGenModule &CGM,
+   const VarDecl *D) const 
{

You should document this functionality. I don't think the dialects will need to 
call this function.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:10035
+  if (CGM.isTypeConstant(D->getType(), false)) {
+if (auto ConstAS = CGM.getTarget().getConstantAddressSpace())
+  return ConstAS.getValue();

You should not have constant address space in SYCL!


Repository:
  rG LLVM Github Monorepo

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

https:/

[clang] acd4950 - [FPEnv] Correct constrained metadata in fp16-ops-strict.c

[Kevin P. Neal via cfe-commits]

Author: Kevin P. Neal
Date: 2020-12-08T10:18:32-05:00
New Revision: acd4950d4f1e4ef5375a8a894a5b359caf7e844b

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

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

This test shows we're in some cases not getting strictfp information from
the AST. Correct that.

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

Added: 


Modified: 
clang/lib/CodeGen/CGExprScalar.cpp
clang/test/CodeGen/fp16-ops-strictfp.c

Removed: 




diff  --git a/clang/lib/CodeGen/CGExprScalar.cpp 
b/clang/lib/CodeGen/CGExprScalar.cpp
index 4210e810acb7..973cefd831e6 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -2550,6 +2550,7 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const 
UnaryOperator *E, LValue LV,
   } else if (type->isRealFloatingType()) {
 // Add the inc/dec to the real part.
 llvm::Value *amt;
+CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, E);
 
 if (type->isHalfType() && !CGF.getContext().getLangOpts().NativeHalfType) {
   // Another special case: half FP increment should be done via float
@@ -3001,6 +3002,7 @@ LValue ScalarExprEmitter::EmitCompoundAssignLValue(
   else
 OpInfo.LHS = EmitLoadOfLValue(LHSLV, E->getExprLoc());
 
+  CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, OpInfo.FPFeatures);
   SourceLocation Loc = E->getExprLoc();
   OpInfo.LHS =
   EmitScalarConversion(OpInfo.LHS, LHSTy, E->getComputationLHSType(), Loc);

diff  --git a/clang/test/CodeGen/fp16-ops-strictfp.c 
b/clang/test/CodeGen/fp16-ops-strictfp.c
index fd50d56a852c..d81febad0c36 100644
--- a/clang/test/CodeGen/fp16-ops-strictfp.c
+++ b/clang/test/CodeGen/fp16-ops-strictfp.c
@@ -11,7 +11,6 @@
 //
 // Test that the constrained intrinsics are picking up the exception
 // metadata from the AST instead of the global default from the command line.
-// FIXME: All cases of "fpexcept.maytrap" in this test are wrong.
 
 #pragma float_control(except, on)
 
@@ -62,31 +61,31 @@ void foo(void) {
   // NOTNATIVE: store {{.*}} half {{.*}}, half*
   h1 = +h1;
 
-  // NATIVE-HALF: call half @llvm.experimental.constrained.fadd.f16(half 
%{{.*}}, half 0xH3C00, metadata !"round.tonearest", metadata 
!"fpexcept.maytrap")
-  // NOTNATIVE: call float @llvm.experimental.constrained.fpext.f32.f16(half 
%{{.*}}, metadata !"fpexcept.maytrap")
-  // NOTNATIVE: call float @llvm.experimental.constrained.fadd.f32(float 
%{{.*}}, float {{.*}}, metadata !"round.tonearest", metadata 
!"fpexcept.maytrap")
-  // NOTNATIVE: call half @llvm.experimental.constrained.fptrunc.f16.f32(float 
%{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
+  // NATIVE-HALF: call half @llvm.experimental.constrained.fadd.f16(half 
%{{.*}}, half 0xH3C00, metadata !"round.tonearest", metadata !"fpexcept.strict")
+  // NOTNATIVE: call float @llvm.experimental.constrained.fpext.f32.f16(half 
%{{.*}}, metadata !"fpexcept.strict")
+  // NOTNATIVE: call float @llvm.experimental.constrained.fadd.f32(float 
%{{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+  // NOTNATIVE: call half @llvm.experimental.constrained.fptrunc.f16.f32(float 
%{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
   // CHECK: store {{.*}} half {{.*}}, half*
   h1++;
 
-  // NATIVE-HALF: call half @llvm.experimental.constrained.fadd.f16(half 
%{{.*}}, half 0xH3C00, metadata !"round.tonearest", metadata 
!"fpexcept.maytrap")
-  // NOTNATIVE: call float @llvm.experimental.constrained.fpext.f32.f16(half 
%{{.*}}, metadata !"fpexcept.maytrap")
-  // NOTNATIVE: call float @llvm.experimental.constrained.fadd.f32(float 
%{{.*}}, float {{.*}}, metadata !"round.tonearest", metadata 
!"fpexcept.maytrap")
-  // NOTNATIVE: call half @llvm.experimental.constrained.fptrunc.f16.f32(float 
%{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
+  // NATIVE-HALF: call half @llvm.experimental.constrained.fadd.f16(half 
%{{.*}}, half 0xH3C00, metadata !"round.tonearest", metadata !"fpexcept.strict")
+  // NOTNATIVE: call float @llvm.experimental.constrained.fpext.f32.f16(half 
%{{.*}}, metadata !"fpexcept.strict")
+  // NOTNATIVE: call float @llvm.experimental.constrained.fadd.f32(float 
%{{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+  // NOTNATIVE: call half @llvm.experimental.constrained.fptrunc.f16.f32(float 
%{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
   // CHECK: store {{.*}} half {{.*}}, half*
   ++h1;
 
-  // NATIVE-HALF: call half @llvm.experimental.constrained.fadd.f16(half 
%{{.*}}, half 0xHBC00, metadata !"round.tonearest", metadata 
!"fpexcept.maytrap")
-  // NOTNATIVE: call float @llvm.experimental.constrained.fpext.f32.f16(half 
%{{.*}}, meta

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

[Kevin P. Neal 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 rGacd4950d4f1e: [FPEnv] Correct constrained metadata in 
fp16-ops-strict.c (authored by kpn).

Changed prior to commit:
  https://reviews.llvm.org/D92596?vs=309327&id=310192#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92596

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/fp16-ops-strictfp.c

Index: clang/test/CodeGen/fp16-ops-strictfp.c
===
--- clang/test/CodeGen/fp16-ops-strictfp.c
+++ clang/test/CodeGen/fp16-ops-strictfp.c
@@ -11,7 +11,6 @@
 //
 // Test that the constrained intrinsics are picking up the exception
 // metadata from the AST instead of the global default from the command line.
-// FIXME: All cases of "fpexcept.maytrap" in this test are wrong.
 
 #pragma float_control(except, on)
 
@@ -62,31 +61,31 @@
   // NOTNATIVE: store {{.*}} half {{.*}}, half*
   h1 = +h1;
 
-  // NATIVE-HALF: call half @llvm.experimental.constrained.fadd.f16(half %{{.*}}, half 0xH3C00, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
-  // NOTNATIVE: call float @llvm.experimental.constrained.fpext.f32.f16(half %{{.*}}, metadata !"fpexcept.maytrap")
-  // NOTNATIVE: call float @llvm.experimental.constrained.fadd.f32(float %{{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
-  // NOTNATIVE: call half @llvm.experimental.constrained.fptrunc.f16.f32(float %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
+  // NATIVE-HALF: call half @llvm.experimental.constrained.fadd.f16(half %{{.*}}, half 0xH3C00, metadata !"round.tonearest", metadata !"fpexcept.strict")
+  // NOTNATIVE: call float @llvm.experimental.constrained.fpext.f32.f16(half %{{.*}}, metadata !"fpexcept.strict")
+  // NOTNATIVE: call float @llvm.experimental.constrained.fadd.f32(float %{{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+  // NOTNATIVE: call half @llvm.experimental.constrained.fptrunc.f16.f32(float %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
   // CHECK: store {{.*}} half {{.*}}, half*
   h1++;
 
-  // NATIVE-HALF: call half @llvm.experimental.constrained.fadd.f16(half %{{.*}}, half 0xH3C00, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
-  // NOTNATIVE: call float @llvm.experimental.constrained.fpext.f32.f16(half %{{.*}}, metadata !"fpexcept.maytrap")
-  // NOTNATIVE: call float @llvm.experimental.constrained.fadd.f32(float %{{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
-  // NOTNATIVE: call half @llvm.experimental.constrained.fptrunc.f16.f32(float %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
+  // NATIVE-HALF: call half @llvm.experimental.constrained.fadd.f16(half %{{.*}}, half 0xH3C00, metadata !"round.tonearest", metadata !"fpexcept.strict")
+  // NOTNATIVE: call float @llvm.experimental.constrained.fpext.f32.f16(half %{{.*}}, metadata !"fpexcept.strict")
+  // NOTNATIVE: call float @llvm.experimental.constrained.fadd.f32(float %{{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+  // NOTNATIVE: call half @llvm.experimental.constrained.fptrunc.f16.f32(float %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
   // CHECK: store {{.*}} half {{.*}}, half*
   ++h1;
 
-  // NATIVE-HALF: call half @llvm.experimental.constrained.fadd.f16(half %{{.*}}, half 0xHBC00, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
-  // NOTNATIVE: call float @llvm.experimental.constrained.fpext.f32.f16(half %{{.*}}, metadata !"fpexcept.maytrap")
-  // NOTNATIVE: call float @llvm.experimental.constrained.fadd.f32(float %{{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
-  // NOTNATIVE: call half @llvm.experimental.constrained.fptrunc.f16.f32(float %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
+  // NATIVE-HALF: call half @llvm.experimental.constrained.fadd.f16(half %{{.*}}, half 0xHBC00, metadata !"round.tonearest", metadata !"fpexcept.strict")
+  // NOTNATIVE: call float @llvm.experimental.constrained.fpext.f32.f16(half %{{.*}}, metadata !"fpexcept.strict")
+  // NOTNATIVE: call float @llvm.experimental.constrained.fadd.f32(float %{{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+  // NOTNATIVE: call half @llvm.experimental.constrained.fptrunc.f16.f32(float %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
   // CHECK: store {{.*}} half {{.*}}, half*
   --h1;
 
-  // NATIVE-HALF: call half @llvm.experimental.constrained.fadd.f16(half %{{.*}}, half 0xHBC00, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
-  // NOTNATIVE: call float @llvm.experimental.constrained.fpext.f32.f16(half %{{.*}}, metadata !"fpexcept.maytrap

[clang-tools-extra] 8d2c095 - [clang-tidy] Omit std::make_unique/make_shared for default initialization.

[Chris Kennelly via cfe-commits]

Author: Chris Kennelly
Date: 2020-12-08T10:34:17-05:00
New Revision: 8d2c095e5a6bd34f8bb5cffd5c57c8deea5b8647

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

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

This extends the check for default initialization in arrays added in
547f89d6070 to include scalar types and exclude them from the suggested fix for
make_unique/make_shared.

Rewriting std::unique_ptr(new int) as std::make_unique() (or for
other, similar trivial T) switches from default initialization to value
initialization, a performance regression for trivial T.  For these use cases,
std::make_unique_for_overwrite is more suitable alternative.

Reviewed By: hokein

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

Added: 

clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique-default-init.cpp

Modified: 
clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h
clang-tools-extra/docs/clang-tidy/checks/modernize-make-shared.rst
clang-tools-extra/docs/clang-tidy/checks/modernize-make-unique.rst
clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp
clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
index c00067fa8257..a7aaca1de5bd 100644
--- a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -6,6 +6,7 @@
 //
 
//===--===//
 
+#include "../utils/TypeTraits.h"
 #include "MakeSharedCheck.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/Lexer.h"
@@ -49,13 +50,17 @@ MakeSmartPtrCheck::MakeSmartPtrCheck(StringRef Name, 
ClangTidyContext *Context,
   Options.get("MakeSmartPtrFunctionHeader", "")),
   MakeSmartPtrFunctionName(
   Options.get("MakeSmartPtrFunction", MakeSmartPtrFunctionName)),
-  IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
+  IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)),
+  IgnoreDefaultInitialization(
+  Options.get("IgnoreDefaultInitialization", true)) {}
 
 void MakeSmartPtrCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "IncludeStyle", Inserter.getStyle());
   Options.store(Opts, "MakeSmartPtrFunctionHeader", 
MakeSmartPtrFunctionHeader);
   Options.store(Opts, "MakeSmartPtrFunction", MakeSmartPtrFunctionName);
   Options.store(Opts, "IgnoreMacros", IgnoreMacros);
+  Options.store(Opts, "IgnoreDefaultInitialization",
+IgnoreDefaultInitialization);
 }
 
 bool MakeSmartPtrCheck::isLanguageVersionSupported(
@@ -120,14 +125,18 @@ void MakeSmartPtrCheck::check(const 
MatchFinder::MatchResult &Result) {
   if (New->getType()->getPointeeType()->getContainedAutoType())
 return;
 
-  // Be conservative for cases where we construct an array without any
-  // initialization.
+  // Be conservative for cases where we construct and default initialize.
+  //
   // For example,
+  //P.reset(new int)// check fix: P = std::make_unique()
   //P.reset(new int[5]) // check fix: P = std::make_unique(5)
   //
-  // The fix of the check has side effect, it introduces default initialization
+  // The fix of the check has side effect, it introduces value initialization
   // which maybe unexpected and cause performance regression.
-  if (New->isArray() && !New->hasInitializer())
+  bool Initializes = New->hasInitializer() ||
+ !utils::type_traits::isTriviallyDefaultConstructible(
+ New->getAllocatedType(), *Result.Context);
+  if (!Initializes && IgnoreDefaultInitialization)
 return;
   if (Construct)
 checkConstruct(SM, Result.Context, Construct, Type, New);

diff  --git a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h 
b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h
index 7a1bba624c53..ca12d7734162 100644
--- a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h
+++ b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h
@@ -50,6 +50,7 @@ class MakeSmartPtrCheck : public ClangTidyCheck {
   const std::string MakeSmartPtrFunctionHeader;
   const std::string MakeSmartPtrFunctionName;
   const bool IgnoreMacros;
+  const bool IgnoreDefaultInitialization;
 
   void checkConstruct(SourceManager &SM, ASTContext *Ctx,
   const CXXConstructExpr *Construct, const QualType *Type,

diff  --git 
a/clang-tools-extra/docs/clang-tidy/checks/modernize-make-shared.rst 
b/clang-

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

[Chris Kennelly via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8d2c095e5a6b: [clang-tidy] Omit std::make_unique/make_shared 
for default initialization. (authored by ckennelly).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90392

Files:
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h
  clang-tools-extra/docs/clang-tidy/checks/modernize-make-shared.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-make-unique.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique-default-init.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique.cpp
@@ -83,49 +83,60 @@
   // CHECK-FIXES: return std::make_unique();
 }
 
+std::unique_ptr getPointerValue() {
+  return std::unique_ptr(new Base());
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use std::make_unique instead
+  // CHECK-FIXES: return std::make_unique();
+}
+
 void basic() {
   std::unique_ptr P1 = std::unique_ptr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: std::unique_ptr P1 = std::make_unique();
+  std::unique_ptr P2 = std::unique_ptr(new int);
 
   P1.reset(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: P1 = std::make_unique();
+  P2.reset(new int);
 
   P1 = std::unique_ptr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: P1 = std::make_unique();
+  P1 = std::unique_ptr(new int);
 
-  // Without parenthesis.
-  std::unique_ptr P2 = std::unique_ptr(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: std::unique_ptr P2 = std::make_unique();
+  // Without parenthesis, default initialization.
+  std::unique_ptr P3 = std::unique_ptr(new int);
 
   P2.reset(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: P2 = std::make_unique();
 
   P2 = std::unique_ptr(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: P2 = std::make_unique();
 
   // With auto.
-  auto P3 = std::unique_ptr(new int());
+  auto P4 = std::unique_ptr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead
-  // CHECK-FIXES: auto P3 = std::make_unique();
+  // CHECK-FIXES: auto P4 = std::make_unique();
+  auto P5 = std::unique_ptr(new int);
 
-  std::unique_ptr P4 = std::unique_ptr((new int));
+  std::unique_ptr P6 = std::unique_ptr((new int()));
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: std::unique_ptr P4 = std::make_unique();
-  P4.reset((new int));
+  // CHECK-FIXES: std::unique_ptr P6 = std::make_unique();
+  std::unique_ptr P7 = std::unique_ptr((new int));
+
+  P4.reset((new int()));
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: P4 = std::make_unique();
-  std::unique_ptr P5 = std::unique_ptrnew int;
+  P5.reset((new int));
+
+  std::unique_ptr P8 = std::unique_ptrnew int();
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: std::unique_ptr P5 = std::make_unique();
-  P5.reset(new int);
+  // CHECK-FIXES: std::unique_ptr P8 = std::make_unique();
+  std::unique_ptr P9 = std::unique_ptrnew int;
+
+  P5.reset(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: P5 = std::make_unique();
+  P6.reset(new int);
 
   {
 // No std.
@@ -133,40 +144,55 @@
 unique_ptr Q = unique_ptr(new int());
 // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use std::make_unique instead
 // CHECK-FIXES: unique_ptr Q = std::make_unique();
+unique_ptr P = unique_ptr(new int);
 
 Q = unique_ptr(new int());
 // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_unique instead
 // CHECK-FIXES: Q = std::make_unique();
+
+P = unique_ptr(new int);
   }
 
   std::unique_ptr R(new int());
+  std::unique_ptr S(new int);
 
  

[PATCH] D91949: [clang-format] Add BeforeStructInitialization option in BraceWrapping configuration

[Anastasiia Lukianenko via Phabricator via cfe-commits]

anastasiia_lukianenko added a comment.

AfterStruct setting doesn't affect struct initialization. Your presumption is 
right.
About these examples... I think that second and third variants are the right 
ones.

  struct new_struct struct_name = {
  a = 1
  };
  struct new_struct struct_name = { a = 1 };
  struct new_struct struct_name = 
  { 
  a = 1,
  };

There is a test which shows that first example is not expected in case without 
break after '='

  TEST_F(FormatTest, UnderstandContextOfRecordTypeKeywords) {
// Elaborate type variable declarations.
verifyFormat("struct foo a = {bar};\nint n;");


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

https://reviews.llvm.org/D91949

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

[PATCH] D92847: [clangd] ExpandAutoType: Do not offer code action on lambdas.

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz created this revision.
Herald added subscribers: usaxena95, kadircet, arphaman.
adamcz requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

We can't expand lambda types anyway. Now we simply not offer the code
action instead of showing it and then returning an error in apply().


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92847

Files:
  clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
  clang-tools-extra/clangd/test/check-fail.test
  clang-tools-extra/clangd/unittests/TweakTests.cpp


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -559,8 +559,7 @@
   EXPECT_THAT(apply("au^to x = &ns::Func;"),
   StartsWith("fail: Could not expand type of function pointer"));
   // lambda types are not replaced
-  EXPECT_THAT(apply("au^to x = []{};"),
-  StartsWith("fail: Could not expand type of lambda expression"));
+  EXPECT_UNAVAILABLE("au^to x = []{};");
   // inline namespaces
   EXPECT_EQ(apply("au^to x = inl_ns::Visible();"),
 "Visible x = inl_ns::Visible();");
Index: clang-tools-extra/clangd/test/check-fail.test
===
--- clang-tools-extra/clangd/test/check-fail.test
+++ clang-tools-extra/clangd/test/check-fail.test
@@ -11,4 +11,5 @@
 // CHECK: All checks completed, 2 errors
 
 #include "missing.h"
-auto x = []{};
+void fun();
+auto x = fun;
Index: clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
@@ -74,6 +74,31 @@
   CachedLocation = Result;
   }
 }
+
+if (!CachedLocation)
+  return false;
+
+// apply() below will use RecursiveASTVisitor to figure out the deduced
+// type. That takes longer, but will handle all cases.
+// This scan of parents here only handles one case (RecordDecl) and we only
+// do it to prevent offering this tweak on lambda types (common use case 
for
+// 'auto'), since it would fail in apply() anyway.
+//
+// In the future, We could re-use this node we found here in apply(), but a
+// better approach would be to fix getDeducedType() to not require walking
+// the entire file and just run in in prepare() instead.
+for (auto *It = Node; It; It = It->Parent) {
+  if (auto *DD = It->ASTNode.get()) {
+if (DD->getTypeSourceInfo() &&
+DD->getTypeSourceInfo()->getTypeLoc().getBeginLoc() ==
+CachedLocation->getBeginLoc()) {
+  if (auto *RD = DD->getType()->getAsRecordDecl())
+if (RD->isLambda())
+  return false;
+  break;
+}
+  }
+}
   }
   return (bool) CachedLocation;
 }


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -559,8 +559,7 @@
   EXPECT_THAT(apply("au^to x = &ns::Func;"),
   StartsWith("fail: Could not expand type of function pointer"));
   // lambda types are not replaced
-  EXPECT_THAT(apply("au^to x = []{};"),
-  StartsWith("fail: Could not expand type of lambda expression"));
+  EXPECT_UNAVAILABLE("au^to x = []{};");
   // inline namespaces
   EXPECT_EQ(apply("au^to x = inl_ns::Visible();"),
 "Visible x = inl_ns::Visible();");
Index: clang-tools-extra/clangd/test/check-fail.test
===
--- clang-tools-extra/clangd/test/check-fail.test
+++ clang-tools-extra/clangd/test/check-fail.test
@@ -11,4 +11,5 @@
 // CHECK: All checks completed, 2 errors
 
 #include "missing.h"
-auto x = []{};
+void fun();
+auto x = fun;
Index: clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
@@ -74,6 +74,31 @@
   CachedLocation = Result;
   }
 }
+
+if (!CachedLocation)
+  return false;
+
+// apply() below will use RecursiveASTVisitor to figure out the deduced
+// type. That takes longer, but will handle all cases.
+// This scan of parents here only handles one case (RecordDecl) and we only
+// do it to prevent offering this tweak on lambda types (common use case for
+// 'auto'), since it would fail in apply() anyway.
+//
+// In the future, We could re-use this node we found here in apply(), but a
+// better approach wou

[PATCH] D92721: [PATCH] [clang] Create SPIRABIInfo to enable SPIR_FUNC calling convention

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D92721#2437941 , @mibintc wrote:

> There are many RuntimeCalls that are created in Clang, including _read_pipe, 
> all the OpenMP runtime calls, etc.  Shall they all have their calling 
> conventions set from the ABIInfo?  Currently those calls are being set to 0.

Makes sense to apply this to all functions indeed, btw there is also 
`getDefaultCallingConvention` from SPIR that could be used...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92721

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

[PATCH] D92231: [OpenCL] Implement extended subgroups fully in headers

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/lib/Headers/opencl-c-base.h:17-23
+#define cl_khr_subgroup_extended_types
+#define cl_khr_subgroup_non_uniform_vote
+#define cl_khr_subgroup_ballot
+#define cl_khr_subgroup_non_uniform_arithmetic
+#define cl_khr_subgroup_shuffle
+#define cl_khr_subgroup_shuffle_relative
+#define cl_khr_subgroup_clustered_reduce

PiotrFusik wrote:
> These are currently defined as "1": https://godbolt.org/z/MnoWeo
> Is the change to blank intentional?
> This should be tested.
Thanks! I think the spec doesn't specify the values but only says that the 
macros are defined

> Every extension which affects the OpenCL language semantics, syntax or adds 
> built-in functions tothe  language  must  create  a  preprocessor  #define  
> that  matches  the  extension  name  string.  This #define  would  be  
> available  in  the  language  if  and  only  if  the  extension  is  
> supported  on  a  givenimplementation.

However, I think it makes sense to set the value `1` to align with the other 
extensions that are added by clang.


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

https://reviews.llvm.org/D92231

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

[PATCH] D92487: [AArch64][Driver][SVE] Push missing SVE feature error from driver to frontend

[Peter Waller via Phabricator via cfe-commits]

peterwaller-arm updated this revision to Diff 310215.
peterwaller-arm added a comment.

Update patch for clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92487

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Driver/aarch64-sve-vector-bits.c
  clang/test/Sema/arm-vector-types-support.c
  clang/test/Sema/neon-vector-types-support.c

Index: clang/test/Sema/neon-vector-types-support.c
===
--- clang/test/Sema/neon-vector-types-support.c
+++ /dev/null
@@ -1,4 +0,0 @@
-// RUN: %clang_cc1 %s -triple armv7 -fsyntax-only -verify
-
-typedef __attribute__((neon_vector_type(2))) int int32x2_t; // expected-error{{'neon_vector_type' attribute is not supported for this target}}
-typedef __attribute__((neon_polyvector_type(16))) short poly8x16_t; // expected-error{{'neon_polyvector_type' attribute is not supported for this target}}
Index: clang/test/Sema/arm-vector-types-support.c
===
--- /dev/null
+++ clang/test/Sema/arm-vector-types-support.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 %s -triple armv7 -fsyntax-only -verify
+
+typedef __attribute__((neon_vector_type(2))) int int32x2_t; // expected-error{{'neon_vector_type' attribute is not supported on targets missing 'neon' or 'mve'; specify an appropriate -march= or -mcpu=}}
+typedef __attribute__((neon_polyvector_type(16))) short poly8x16_t; // expected-error{{'neon_polyvector_type' attribute is not supported on targets missing 'neon' or 'mve'; specify an appropriate -march= or -mcpu=}}
+typedef __attribute__((arm_sve_vector_bits(256))) void nosveflag; // expected-error{{'arm_sve_vector_bits' attribute is not supported on targets missing 'sve'; specify an appropriate -march= or -mcpu=}}
Index: clang/test/Driver/aarch64-sve-vector-bits.c
===
--- clang/test/Driver/aarch64-sve-vector-bits.c
+++ clang/test/Driver/aarch64-sve-vector-bits.c
@@ -22,21 +22,6 @@
 // CHECK-2048: "-msve-vector-bits=2048"
 // CHECK-SCALABLE-NOT: "-msve-vector-bits=
 
-// Bail out if -msve-vector-bits is specified without SVE enabled
-// -
-// RUN: %clang -c %s -### -target aarch64-none-linux-gnu -msve-vector-bits=128 \
-// RUN:  2>&1 | FileCheck --check-prefix=CHECK-NO-SVE-ERROR %s
-// RUN: %clang -c %s -### -target aarch64-none-linux-gnu -msve-vector-bits=256 \
-// RUN:  2>&1 | FileCheck --check-prefix=CHECK-NO-SVE-ERROR %s
-// RUN: %clang -c %s -### -target aarch64-none-linux-gnu -msve-vector-bits=512 \
-// RUN:  2>&1 | FileCheck --check-prefix=CHECK-NO-SVE-ERROR %s
-// RUN: %clang -c %s -### -target aarch64-none-linux-gnu -msve-vector-bits=1024 \
-// RUN:  2>&1 | FileCheck --check-prefix=CHECK-NO-SVE-ERROR %s
-// RUN: %clang -c %s -### -target aarch64-none-linux-gnu -msve-vector-bits=2048 \
-// RUN:  2>&1 | FileCheck --check-prefix=CHECK-NO-SVE-ERROR %s
-
-// CHECK-NO-SVE-ERROR: error: '-msve-vector-bits' is not supported without SVE enabled
-
 // Error out if an unsupported value is passed to -msve-vector-bits.
 // -
 // RUN: %clang -c %s -### -target aarch64-none-linux-gnu -march=armv8-a+sve \
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -7799,7 +7799,8 @@
   // not to need a separate attribute)
   if (!S.Context.getTargetInfo().hasFeature("neon") &&
   !S.Context.getTargetInfo().hasFeature("mve")) {
-S.Diag(Attr.getLoc(), diag::err_attribute_unsupported) << Attr;
+S.Diag(Attr.getLoc(), diag::err_attribute_unsupported)
+<< Attr << "'neon' or 'mve'";
 Attr.setInvalid();
 return;
   }
@@ -7842,7 +7843,7 @@
Sema &S) {
   // Target must have SVE.
   if (!S.Context.getTargetInfo().hasFeature("sve")) {
-S.Diag(Attr.getLoc(), diag::err_attribute_unsupported) << Attr;
+S.Diag(Attr.getLoc(), diag::err_attribute_unsupported) << Attr << "'sve'";
 Attr.setInvalid();
 return;
   }
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -381,12 +381,6 @@
   if (V8_6Pos != std::end(Features))
 V8_6Pos = Features.insert(std::next(V8_6Pos), {"+i8mm", "+bf16"});
 
-  bool HasSve = llvm::is_contained(Features, "+sve");
-  // -msve-vector-bits= flag is valid only if SVE is enabled.
-  if (Args.hasArg(options::OPT_msve_vector_bits_EQ))
-if (!HasSve)
-  D

[clang] d14c631 - [analyzer][StdLibraryFunctionsChecker] Make close and mmap to accept -1 as fd

[Gabor Marton via cfe-commits]

Author: Gabor Marton
Date: 2020-12-08T16:58:30+01:00
New Revision: d14c63167315edfc4a4ad91fac9c866c6e0cb67f

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

LOG: [analyzer][StdLibraryFunctionsChecker] Make close and mmap to accept -1 as 
fd

close:
It is quite often that users chose to call close even if the fd is
negative. Theoretically, it would be nicer to close only valid fds, but
in practice the implementations of close just returns with EBADF in case
of a non-valid fd param. So, we can eliminate many false positives if we
let close to take -1 as an fd. Other negative values are very unlikely,
because open and other fd factories return with -1 in case of failure.

mmap:
In the case of MAP_ANONYMOUS flag (which is supported e.g. in Linux) the
mapping is not backed by any file; its contents are initialized to zero.
The fd argument is ignored; however, some implementations require fd to
be -1 if MAP_ANONYMOUS (or MAP_ANON) is specified, and portable
applications should ensure this.
Consequently, we must allow -1 as the 4th arg.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 8a34950ce7341..de825b2fee11f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -1672,7 +1672,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 addToFunctionSummaryMap("close", Signature(ArgTypes{IntTy}, 
RetType{IntTy}),
 Summary(NoEvalCall)
 .ArgConstraint(ArgumentCondition(
-0, WithinRange, Range(0, IntMax;
+0, WithinRange, Range(-1, IntMax;
 
 // long fpathconf(int fildes, int name);
 addToFunctionSummaryMap("fpathconf",
@@ -1734,7 +1734,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 Summary(NoEvalCall)
 .ArgConstraint(ArgumentCondition(1, WithinRange, Range(1, 
SizeMax)))
 .ArgConstraint(
-ArgumentCondition(4, WithinRange, Range(0, IntMax;
+ArgumentCondition(4, WithinRange, Range(-1, IntMax;
 
 Optional Off64_tTy = lookupTy("off64_t");
 // void *mmap64(void *addr, size_t length, int prot, int flags, int fd,
@@ -1746,7 +1746,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 Summary(NoEvalCall)
 .ArgConstraint(ArgumentCondition(1, WithinRange, Range(1, 
SizeMax)))
 .ArgConstraint(
-ArgumentCondition(4, WithinRange, Range(0, IntMax;
+ArgumentCondition(4, WithinRange, Range(-1, IntMax;
 
 // int pipe(int fildes[2]);
 addToFunctionSummaryMap(



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

[PATCH] D92764: [analyzer][StdLibraryFunctionsChecker] Make close and mmap to accept -1 as fd

[Gabor Marton 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 rGd14c63167315: [analyzer][StdLibraryFunctionsChecker] Make 
close and mmap to accept -1 as fd (authored by martong).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92764

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -1672,7 +1672,7 @@
 addToFunctionSummaryMap("close", Signature(ArgTypes{IntTy}, 
RetType{IntTy}),
 Summary(NoEvalCall)
 .ArgConstraint(ArgumentCondition(
-0, WithinRange, Range(0, IntMax;
+0, WithinRange, Range(-1, IntMax;
 
 // long fpathconf(int fildes, int name);
 addToFunctionSummaryMap("fpathconf",
@@ -1734,7 +1734,7 @@
 Summary(NoEvalCall)
 .ArgConstraint(ArgumentCondition(1, WithinRange, Range(1, 
SizeMax)))
 .ArgConstraint(
-ArgumentCondition(4, WithinRange, Range(0, IntMax;
+ArgumentCondition(4, WithinRange, Range(-1, IntMax;
 
 Optional Off64_tTy = lookupTy("off64_t");
 // void *mmap64(void *addr, size_t length, int prot, int flags, int fd,
@@ -1746,7 +1746,7 @@
 Summary(NoEvalCall)
 .ArgConstraint(ArgumentCondition(1, WithinRange, Range(1, 
SizeMax)))
 .ArgConstraint(
-ArgumentCondition(4, WithinRange, Range(0, IntMax;
+ArgumentCondition(4, WithinRange, Range(-1, IntMax;
 
 // int pipe(int fildes[2]);
 addToFunctionSummaryMap(


Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -1672,7 +1672,7 @@
 addToFunctionSummaryMap("close", Signature(ArgTypes{IntTy}, RetType{IntTy}),
 Summary(NoEvalCall)
 .ArgConstraint(ArgumentCondition(
-0, WithinRange, Range(0, IntMax;
+0, WithinRange, Range(-1, IntMax;
 
 // long fpathconf(int fildes, int name);
 addToFunctionSummaryMap("fpathconf",
@@ -1734,7 +1734,7 @@
 Summary(NoEvalCall)
 .ArgConstraint(ArgumentCondition(1, WithinRange, Range(1, SizeMax)))
 .ArgConstraint(
-ArgumentCondition(4, WithinRange, Range(0, IntMax;
+ArgumentCondition(4, WithinRange, Range(-1, IntMax;
 
 Optional Off64_tTy = lookupTy("off64_t");
 // void *mmap64(void *addr, size_t length, int prot, int flags, int fd,
@@ -1746,7 +1746,7 @@
 Summary(NoEvalCall)
 .ArgConstraint(ArgumentCondition(1, WithinRange, Range(1, SizeMax)))
 .ArgConstraint(
-ArgumentCondition(4, WithinRange, Range(0, IntMax;
+ArgumentCondition(4, WithinRange, Range(-1, IntMax;
 
 // int pipe(int fildes[2]);
 addToFunctionSummaryMap(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92771: [analyzer][StdLibraryFunctionsChecker] Add more return value contraints

[Gabor Marton via Phabricator via cfe-commits]

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

Thanks for the review!




Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1367-1368
+.Case(ReturnsZeroOrMinusOne)
 .ArgConstraint(ArgumentCondition(
 0, WithinRange, Range(0, IntMax;
 

steakhal wrote:
> I think you should hoist this ArgumentCondition construction with a lambda 
> call. It would lead to more readable summaries.
> 
> ```lang=c++
> const auto ValidFileDescriptorArgAt = [](unsigned ArgIdx) {
>   return ArgumentCondition(ArgIdx, WithinRange, Range(0, IntMax;
> };
> ```
> Probably the very same principle would apply for handling `off_t` arguments.
> 
> You can probably find a better name, but you get the idea.
> If you agree on this, you could create a follow-up patch.
Good idea, thanks! I am going to create another patch for this (and another for 
exec* functions).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92771

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

[PATCH] D92231: [OpenCL] Implement extended subgroups fully in headers

[Piotr Fusik via Phabricator via cfe-commits]

PiotrFusik added inline comments.



Comment at: clang/lib/Headers/opencl-c-base.h:17-23
+#define cl_khr_subgroup_extended_types
+#define cl_khr_subgroup_non_uniform_vote
+#define cl_khr_subgroup_ballot
+#define cl_khr_subgroup_non_uniform_arithmetic
+#define cl_khr_subgroup_shuffle
+#define cl_khr_subgroup_shuffle_relative
+#define cl_khr_subgroup_clustered_reduce

Anastasia wrote:
> PiotrFusik wrote:
> > These are currently defined as "1": https://godbolt.org/z/MnoWeo
> > Is the change to blank intentional?
> > This should be tested.
> Thanks! I think the spec doesn't specify the values but only says that the 
> macros are defined
> 
> > Every extension which affects the OpenCL language semantics, syntax or adds 
> > built-in functions tothe  language  must  create  a  preprocessor  #define  
> > that  matches  the  extension  name  string.  This #define  would  be  
> > available  in  the  language  if  and  only  if  the  extension  is  
> > supported  on  a  givenimplementation.
> 
> However, I think it makes sense to set the value `1` to align with the other 
> extensions that are added by clang.
> I think the spec doesn't specify the values but only says that the macros are 
> defined

Yes, confirmed with Ben:

> I don’t think we’ve said anything about the extension #defines, but for the 
> OpenCL C 3.0 feature test macros we required that they are defined to a value 
> for precisely this reason (#if used instead of #ifdef):

> > In OpenCL C 3.0 or newer, feature macros must expand to the value 1 if the 
> > feature macro is defined by the OpenCL C compiler. A feature macro must not 
> > be defined if the feature is not supported by the OpenCL C compiler. A 
> > feature macro may expand to a different value in the future, but if this 
> > occurs the value of the feature macro must compare greater than the prior 
> > value of the feature macro.



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

https://reviews.llvm.org/D92231

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

[clang] febe750 - [analyzer][StdLibraryFunctionsChecker] Add more return value contraints

[Gabor Marton via cfe-commits]

Author: Gabor Marton
Date: 2020-12-08T17:04:29+01:00
New Revision: febe75032f6f8322cce1dcbba11a44559aaa14e3

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

LOG: [analyzer][StdLibraryFunctionsChecker] Add more return value contraints

This time, we add contraints to functions that either return with [0, -1] or
with a file descriptor.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index de825b2fee11..f0710a658148 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -1322,21 +1322,31 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 .ArgConstraint(ArgumentCondition(
 0, WithinRange, Range(0, LongMax;
 
+const auto ReturnsZeroOrMinusOne =
+ConstraintSet{ReturnValueCondition(WithinRange, Range(-1, 0))};
+const auto ReturnsFileDescriptor =
+ConstraintSet{ReturnValueCondition(WithinRange, Range(-1, IntMax))};
+
 // int access(const char *pathname, int amode);
 addToFunctionSummaryMap(
 "access", Signature(ArgTypes{ConstCharPtrTy, IntTy}, RetType{IntTy}),
-Summary(NoEvalCall).ArgConstraint(NotNull(ArgNo(0;
+Summary(NoEvalCall)
+.Case(ReturnsZeroOrMinusOne)
+.ArgConstraint(NotNull(ArgNo(0;
 
 // int faccessat(int dirfd, const char *pathname, int mode, int flags);
 addToFunctionSummaryMap(
 "faccessat",
 Signature(ArgTypes{IntTy, ConstCharPtrTy, IntTy, IntTy},
   RetType{IntTy}),
-Summary(NoEvalCall).ArgConstraint(NotNull(ArgNo(1;
+Summary(NoEvalCall)
+.Case(ReturnsZeroOrMinusOne)
+.ArgConstraint(NotNull(ArgNo(1;
 
 // int dup(int fildes);
 addToFunctionSummaryMap("dup", Signature(ArgTypes{IntTy}, RetType{IntTy}),
 Summary(NoEvalCall)
+.Case(ReturnsFileDescriptor)
 .ArgConstraint(ArgumentCondition(
 0, WithinRange, Range(0, IntMax;
 
@@ -1344,6 +1354,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 addToFunctionSummaryMap(
 "dup2", Signature(ArgTypes{IntTy, IntTy}, RetType{IntTy}),
 Summary(NoEvalCall)
+.Case(ReturnsFileDescriptor)
 .ArgConstraint(ArgumentCondition(0, WithinRange, Range(0, IntMax)))
 .ArgConstraint(
 ArgumentCondition(1, WithinRange, Range(0, IntMax;
@@ -1352,6 +1363,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 addToFunctionSummaryMap("fdatasync",
 Signature(ArgTypes{IntTy}, RetType{IntTy}),
 Summary(NoEvalCall)
+.Case(ReturnsZeroOrMinusOne)
 .ArgConstraint(ArgumentCondition(
 0, WithinRange, Range(0, IntMax;
 
@@ -1367,6 +1379,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 // int fsync(int fildes);
 addToFunctionSummaryMap("fsync", Signature(ArgTypes{IntTy}, 
RetType{IntTy}),
 Summary(NoEvalCall)
+.Case(ReturnsZeroOrMinusOne)
 .ArgConstraint(ArgumentCondition(
 0, WithinRange, Range(0, IntMax;
 
@@ -1376,13 +1389,16 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 addToFunctionSummaryMap(
 "truncate",
 Signature(ArgTypes{ConstCharPtrTy, Off_tTy}, RetType{IntTy}),
-Summary(NoEvalCall).ArgConstraint(NotNull(ArgNo(0;
+Summary(NoEvalCall)
+.Case(ReturnsZeroOrMinusOne)
+.ArgConstraint(NotNull(ArgNo(0;
 
 // int symlink(const char *oldpath, const char *newpath);
 addToFunctionSummaryMap(
 "symlink",
 Signature(ArgTypes{ConstCharPtrTy, ConstCharPtrTy}, RetType{IntTy}),
 Summary(NoEvalCall)
+.Case(ReturnsZeroOrMinusOne)
 .ArgConstraint(NotNull(ArgNo(0)))
 .ArgConstraint(NotNull(ArgNo(1;
 
@@ -1392,6 +1408,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 Signature(ArgTypes{ConstCharPtrTy, IntTy, ConstCharPtrTy},
   RetType{IntTy}),
 Summary(NoEvalCall)
+.Case(ReturnsZeroOrMinusOne)
 .ArgConstraint(NotNul

[PATCH] D92771: [analyzer][StdLibraryFunctionsChecker] Add more return value contraints

[Gabor Marton via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
martong marked an inline comment as done.
Closed by commit rGfebe75032f6f: [analyzer][StdLibraryFunctionsChecker] Add 
more return value contraints (authored by martong).

Changed prior to commit:
  https://reviews.llvm.org/D92771?vs=309940&id=310222#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92771

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -1322,21 +1322,31 @@
 .ArgConstraint(ArgumentCondition(
 0, WithinRange, Range(0, LongMax;
 
+const auto ReturnsZeroOrMinusOne =
+ConstraintSet{ReturnValueCondition(WithinRange, Range(-1, 0))};
+const auto ReturnsFileDescriptor =
+ConstraintSet{ReturnValueCondition(WithinRange, Range(-1, IntMax))};
+
 // int access(const char *pathname, int amode);
 addToFunctionSummaryMap(
 "access", Signature(ArgTypes{ConstCharPtrTy, IntTy}, RetType{IntTy}),
-Summary(NoEvalCall).ArgConstraint(NotNull(ArgNo(0;
+Summary(NoEvalCall)
+.Case(ReturnsZeroOrMinusOne)
+.ArgConstraint(NotNull(ArgNo(0;
 
 // int faccessat(int dirfd, const char *pathname, int mode, int flags);
 addToFunctionSummaryMap(
 "faccessat",
 Signature(ArgTypes{IntTy, ConstCharPtrTy, IntTy, IntTy},
   RetType{IntTy}),
-Summary(NoEvalCall).ArgConstraint(NotNull(ArgNo(1;
+Summary(NoEvalCall)
+.Case(ReturnsZeroOrMinusOne)
+.ArgConstraint(NotNull(ArgNo(1;
 
 // int dup(int fildes);
 addToFunctionSummaryMap("dup", Signature(ArgTypes{IntTy}, RetType{IntTy}),
 Summary(NoEvalCall)
+.Case(ReturnsFileDescriptor)
 .ArgConstraint(ArgumentCondition(
 0, WithinRange, Range(0, IntMax;
 
@@ -1344,6 +1354,7 @@
 addToFunctionSummaryMap(
 "dup2", Signature(ArgTypes{IntTy, IntTy}, RetType{IntTy}),
 Summary(NoEvalCall)
+.Case(ReturnsFileDescriptor)
 .ArgConstraint(ArgumentCondition(0, WithinRange, Range(0, IntMax)))
 .ArgConstraint(
 ArgumentCondition(1, WithinRange, Range(0, IntMax;
@@ -1352,6 +1363,7 @@
 addToFunctionSummaryMap("fdatasync",
 Signature(ArgTypes{IntTy}, RetType{IntTy}),
 Summary(NoEvalCall)
+.Case(ReturnsZeroOrMinusOne)
 .ArgConstraint(ArgumentCondition(
 0, WithinRange, Range(0, IntMax;
 
@@ -1367,6 +1379,7 @@
 // int fsync(int fildes);
 addToFunctionSummaryMap("fsync", Signature(ArgTypes{IntTy}, RetType{IntTy}),
 Summary(NoEvalCall)
+.Case(ReturnsZeroOrMinusOne)
 .ArgConstraint(ArgumentCondition(
 0, WithinRange, Range(0, IntMax;
 
@@ -1376,13 +1389,16 @@
 addToFunctionSummaryMap(
 "truncate",
 Signature(ArgTypes{ConstCharPtrTy, Off_tTy}, RetType{IntTy}),
-Summary(NoEvalCall).ArgConstraint(NotNull(ArgNo(0;
+Summary(NoEvalCall)
+.Case(ReturnsZeroOrMinusOne)
+.ArgConstraint(NotNull(ArgNo(0;
 
 // int symlink(const char *oldpath, const char *newpath);
 addToFunctionSummaryMap(
 "symlink",
 Signature(ArgTypes{ConstCharPtrTy, ConstCharPtrTy}, RetType{IntTy}),
 Summary(NoEvalCall)
+.Case(ReturnsZeroOrMinusOne)
 .ArgConstraint(NotNull(ArgNo(0)))
 .ArgConstraint(NotNull(ArgNo(1;
 
@@ -1392,6 +1408,7 @@
 Signature(ArgTypes{ConstCharPtrTy, IntTy, ConstCharPtrTy},
   RetType{IntTy}),
 Summary(NoEvalCall)
+.Case(ReturnsZeroOrMinusOne)
 .ArgConstraint(NotNull(ArgNo(0)))
 .ArgConstraint(ArgumentCondition(1, WithinRange, Range(0, IntMax)))
 .ArgConstraint(NotNull(ArgNo(2;
@@ -1400,6 +1417,7 @@
 addToFunctionSummaryMap(
 "lockf", Signature(ArgTypes{IntTy, IntTy, Off_tTy}, RetType{IntTy}),
 Summary(NoEvalCall)
+.Case(ReturnsZeroOrMinusOne)
 .ArgConstraint(
 ArgumentCondition(0, WithinRange, Range(0, IntMax;
 
@@ -1408,7 +1426,9 @@
 // int creat(const char *pathname, mode_t m

[PATCH] D92231: [OpenCL] Implement extended subgroups fully in headers

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 310227.
Anastasia added a comment.

- Set all macros to value 1


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

https://reviews.llvm.org/D92231

Files:
  clang/include/clang/Basic/OpenCLExtensions.def
  clang/lib/Headers/opencl-c-base.h
  clang/test/Headers/opencl-c-header.cl
  clang/test/SemaOpenCL/extension-version.cl

Index: clang/test/SemaOpenCL/extension-version.cl
===
--- clang/test/SemaOpenCL/extension-version.cl
+++ clang/test/SemaOpenCL/extension-version.cl
@@ -205,88 +205,3 @@
 // expected-warning@+2{{unsupported OpenCL extension 'cl_intel_device_side_avc_motion_estimation' - ignoring}}
 #endif
 #pragma OPENCL EXTENSION cl_intel_device_side_avc_motion_estimation : enable
-
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200)
-#ifndef cl_khr_subgroup_extended_types
-#error "Missing cl_khr_subgroup_extended_types"
-#endif
-#else
-#ifdef cl_khr_subgroup_extended_types
-#error "Incorrect cl_khr_subgroup_extended_types define"
-#endif
-// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_subgroup_extended_types' - ignoring}}
-#endif
-#pragma OPENCL EXTENSION cl_khr_subgroup_extended_types : enable
-
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200)
-#ifndef cl_khr_subgroup_non_uniform_vote
-#error "Missing cl_khr_subgroup_non_uniform_vote"
-#endif
-#else
-#ifdef cl_khr_subgroup_non_uniform_vote
-#error "Incorrect cl_khr_subgroup_non_uniform_vote define"
-#endif
-// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_subgroup_non_uniform_vote' - ignoring}}
-#endif
-#pragma OPENCL EXTENSION cl_khr_subgroup_non_uniform_vote : enable
-
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200)
-#ifndef cl_khr_subgroup_ballot
-#error "Missing cl_khr_subgroup_ballot"
-#endif
-#else
-#ifdef cl_khr_subgroup_ballot
-#error "Incorrect cl_khr_subgroup_ballot define"
-#endif
-// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_subgroup_ballot' - ignoring}}
-#endif
-#pragma OPENCL EXTENSION cl_khr_subgroup_ballot : enable
-
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200)
-#ifndef cl_khr_subgroup_non_uniform_arithmetic
-#error "Missing cl_khr_subgroup_non_uniform_arithmetic"
-#endif
-#else
-#ifdef cl_khr_subgroup_non_uniform_arithmetic
-#error "Incorrect cl_khr_subgroup_non_uniform_arithmetic define"
-#endif
-// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_subgroup_non_uniform_arithmetic' - ignoring}}
-#endif
-#pragma OPENCL EXTENSION cl_khr_subgroup_non_uniform_arithmetic : enable
-
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200)
-#ifndef cl_khr_subgroup_shuffle
-#error "Missing cl_khr_subgroup_shuffle"
-#endif
-#else
-#ifdef cl_khr_subgroup_shuffle
-#error "Incorrect cl_khr_subgroup_shuffle define"
-#endif
-// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_subgroup_shuffle' - ignoring}}
-#endif
-#pragma OPENCL EXTENSION cl_khr_subgroup_shuffle : enable
-
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200)
-#ifndef cl_khr_subgroup_shuffle_relative
-#error "Missing cl_khr_subgroup_shuffle_relative"
-#endif
-#else
-#ifdef cl_khr_subgroup_shuffle_relative
-#error "Incorrect cl_khr_subgroup_shuffle_relative define"
-#endif
-// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_subgroup_shuffle_relative' - ignoring}}
-#endif
-#pragma OPENCL EXTENSION cl_khr_subgroup_shuffle_relative : enable
-
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200)
-#ifndef cl_khr_subgroup_clustered_reduce
-#error "Missing cl_khr_subgroup_clustered_reduce"
-#endif
-#else
-#ifdef cl_khr_subgroup_clustered_reduce
-#error "Incorrect cl_khr_subgroup_clustered_reduce define"
-#endif
-// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_subgroup_clustered_reduce' - ignoring}}
-#endif
-#pragma OPENCL EXTENSION cl_khr_subgroup_clustered_reduce : enable
-
Index: clang/test/Headers/opencl-c-header.cl
===
--- clang/test/Headers/opencl-c-header.cl
+++ clang/test/Headers/opencl-c-header.cl
@@ -84,7 +84,11 @@
 #if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0)
 global atomic_int z = ATOMIC_VAR_INIT(99);
 #endif //__OPENCL_C_VERSION__
+// CHECK-MOD: Reading modules
+
+// Check that extension macros are defined correctly.
 
+// FIXME: this should not be defined for all targets
 // Verify that non-builtin cl_intel_planar_yuv extension is defined from
 // OpenCL 1.2 onwards.
 #if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_1_2)
@@ -94,4 +98,57 @@
 #endif //__OPENCL_C_VERSION__
 #pragma OPENCL EXTENSION cl_intel_planar_yuv : enable
 
-// CHECK-MOD: Reading modules
+// For SPIR all extensions are supported.
+#if defined(__SPIR__)
+
+#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200)
+
+#if cl_khr_subgroup_extended_t

[PATCH] D92852: [NFC] Reduce include files dependency and AA header cleanup (part 2).

[Daniil Fukalov via Phabricator via cfe-commits]

dfukalov created this revision.
dfukalov added a reviewer: RKSimon.
Herald added subscribers: kerbowa, asbirlea, jfb, steven_wu, george.burgess.iv, 
zzheng, hiraditya, eraman, nhaehnle, jvesely, arsenm, MatzeB.
dfukalov requested review of this revision.
Herald added projects: clang, LLVM.
Herald added a subscriber: cfe-commits.

Continuing work started in https://reviews.llvm.org/D92489:

1. Removed a bunch of includes from "AliasAnalysis.h" and "LoopPassManager.h".
2. Minor `const` modifiers unifications.
3. Using `AAQueryInfo::IsCapturedCacheT`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92852

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/examples/Bye/Bye.cpp
  llvm/include/llvm/Analysis/AliasAnalysis.h
  llvm/include/llvm/Analysis/BasicAliasAnalysis.h
  llvm/include/llvm/Analysis/MemorySSA.h
  llvm/include/llvm/Transforms/Scalar/LoopPassManager.h
  llvm/lib/Analysis/AliasAnalysis.cpp
  llvm/lib/Analysis/CaptureTracking.cpp
  llvm/lib/Analysis/MemDepPrinter.cpp
  llvm/lib/Analysis/ScopedNoAliasAA.cpp
  llvm/lib/Analysis/TypeBasedAliasAnalysis.cpp
  llvm/lib/CodeGen/LiveIntervals.cpp
  llvm/lib/LTO/Caching.cpp
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Target/AMDGPU/AMDGPUAliasAnalysis.cpp
  llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
  llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
  llvm/lib/Transforms/IPO/FunctionAttrs.cpp
  llvm/lib/Transforms/IPO/HotColdSplitting.cpp
  llvm/lib/Transforms/IPO/Inliner.cpp
  llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
  llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
  llvm/lib/Transforms/ObjCARC/ProvenanceAnalysisEvaluator.cpp
  llvm/lib/Transforms/Scalar/FlattenCFGPass.cpp
  llvm/lib/Transforms/Scalar/Float2Int.cpp
  llvm/lib/Transforms/Scalar/LoopDistribute.cpp
  llvm/lib/Transforms/Scalar/LoopLoadElimination.cpp
  llvm/lib/Transforms/Scalar/LoopPassManager.cpp
  llvm/lib/Transforms/Scalar/LoopSimplifyCFG.cpp
  llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
  llvm/lib/Transforms/Utils/LoopRotationUtils.cpp
  llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp
  llvm/lib/Transforms/Utils/LoopVersioning.cpp
  llvm/tools/llvm-opt-fuzzer/llvm-opt-fuzzer.cpp
  llvm/tools/opt/NewPMDriver.cpp

Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -16,6 +16,7 @@
 #include "PassPrinters.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Analysis/AliasAnalysis.h"
 #include "llvm/Analysis/CGSCCPassManager.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Bitcode/BitcodeWriterPass.h"
Index: llvm/tools/llvm-opt-fuzzer/llvm-opt-fuzzer.cpp
===
--- llvm/tools/llvm-opt-fuzzer/llvm-opt-fuzzer.cpp
+++ llvm/tools/llvm-opt-fuzzer/llvm-opt-fuzzer.cpp
@@ -10,6 +10,7 @@
 //
 //===--===//
 
+#include "llvm/Analysis/AliasAnalysis.h"
 #include "llvm/Bitcode/BitcodeReader.h"
 #include "llvm/Bitcode/BitcodeWriter.h"
 #include "llvm/CodeGen/CommandFlags.h"
@@ -18,6 +19,7 @@
 #include "llvm/IR/Verifier.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Passes/PassBuilder.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/TargetSelect.h"
Index: llvm/lib/Transforms/Utils/LoopVersioning.cpp
===
--- llvm/lib/Transforms/Utils/LoopVersioning.cpp
+++ llvm/lib/Transforms/Utils/LoopVersioning.cpp
@@ -14,7 +14,6 @@
 
 #include "llvm/Transforms/Utils/LoopVersioning.h"
 #include "llvm/ADT/ArrayRef.h"
-#include "llvm/Analysis/AliasAnalysis.h"
 #include "llvm/Analysis/LoopAccessAnalysis.h"
 #include "llvm/Analysis/LoopInfo.h"
 #include "llvm/Analysis/MemorySSA.h"
Index: llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp
===
--- llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp
+++ llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp
@@ -22,7 +22,6 @@
 
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/Statistic.h"
-#include "llvm/Analysis/AliasAnalysis.h"
 #include "llvm/Analysis/LoopIterator.h"
 #include "llvm/Analysis/ScalarEvolution.h"
 #include "llvm/IR/BasicBlock.h"
Index: llvm/lib/Transforms/Utils/LoopRotationUtils.cpp
===
--- llvm/lib/Transforms/Utils/LoopRotationUtils.cpp
+++ llvm/lib/Transforms/Utils/LoopRotationUtils.cpp
@@ -12,7 +12,6 @@
 
 #include "llvm/Transforms/Utils/LoopRotationUtils.h"
 #include "llvm/ADT/Statistic.h"
-#include "llvm/Analysis/AliasAnalysis.h"
 #include "llvm/Analysis/AssumptionCache.h"
 #include "llvm/Analysis/BasicAliasAnalysis.h"
 #include "llvm/Analysis/CodeMetrics.h"
Index: llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
=

[PATCH] D92852: [NFC] Reduce include files dependency and AA header cleanup (part 2).

[Daniil Fukalov via Phabricator via cfe-commits]

dfukalov added inline comments.



Comment at: llvm/include/llvm/Analysis/AliasAnalysis.h:66
 class Value;
+class TargetLibraryInfo;
 

Just realized wrong order, will fix in updated patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92852

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

[PATCH] D92209: [ASTImporter] Support CXXDeductionGuideDecl with local typedef

[Raphael Isemann via Phabricator via cfe-commits]

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

There are several LLDB tests failing with this change. The 
TestImportBaseClassWhenClassHasDerivedMember.py is probably the only relevant 
one. The others are probably failing for the same reason but with a more 
convoluted setup. I looked into the failure and it seems this patch is now 
skipping the workaround in `ImportContext` that was introduced in D78000 
 (as the `Importer.GetAlreadyImportedOrNull` 
will give us a DeclContext and then we just use that as-is). If you remove the 
`if (!DC || !LexicalDC)` before the `if ((Err = ImportDeclContext(D, DC, 
LexicalDC)))` this should keep the old behaviour.

Beside that issue this LGTM. Removing the 'if' seems straightforward, so I'll 
accept this. Thanks!




Comment at: clang/lib/AST/ASTImporter.cpp:2522
+  // Add to the lookupTable because we could not do that in MapImported.
+  Importer.AddToLookupTable(ToTypedef);
+

martong wrote:
> shafik wrote:
> > I am not super excited about this solution, I feel like several bugs we 
> > have had can be attributed to these exception control flow cases that we 
> > have in the ASTImporter. I don't have any immediate better solution.
> > 
> > 
> Before uploading this patch I had been thinking about several other solutions
> but all of them had some problems:
> 
> 1) 
> Detect the loop in the AST and return with UnsupportedConstruct. We could do
> the detection with the help of ImportPath. However, I realized this approach 
> is
> way too defensive and removes the support for an important AST node which is
> against our goals.
> 
> 2) 
> Try to eliminate the looping contruct from the AST. I could do that for some
> cases (D92101) but there are still cases when the Sema must create such a loop
> deliberately (the test case in this patch shows that).
> 
> It is essential to add a newly created Decl to the lookupTable ASAP because it
> makes it possible for subsequent imports to find the existing Decl and this 
> way
> avoiding the creation of duplicated Decls. This is why AddToLookupTable is
> called in MapImported. But the lookuptable operates on the DeclContext of the
> Decl, and in this patch we must not import the DeclContext before.
> Consequently, we have to add to the loopkuptable once the DeclContext is
> imported. Perhaps, we could provide an optional lambda function to
> GetImportedOrCreateDecl which would be called before MapImported (and that
> lambda would do the DC import), but this seems even more clumsy.
> 
> BTW, the lookuptable is not used in LLDB, there you use the traditional lookup
> implemented in DeclContext (addDeclInternal and noload_lookup). One problem
> with noload_lookup is that it does not find some Decls because it obeys to C++
> visibility rules, thus new duplicated Decls are created. The ASTImporter must
> be able to lookup e.g. template specialization Decls too, in order to avoid
> creating a redundant duplicated clone and this is the task of the lookupTable.
> I hope one day LLDB would be able to switch to ASTImporterLookupTable from
> noload_lookup. The other problem is with addDeclInternal: we call this later,
> not in MapImported. The only responsibility attached to the use of 
> addDeclInternal 
> should be to clone the visibility as it is in the source context (and not 
> managing 
> the do-not-create-duplicate-decls issue).
> 
> So yes, there are many exceptional control flow cases, but most of them stems
> from the complexity of trying to support two different needs: noload_lookup 
> and
> minimal import are needed exceptionally for LLDB. I was thinking about to
> create a separate ASTImporter implementation specifically for CTU and LLDB
> could have it's own implementation. For that we just need to create an
> interface class with virtual functions. Having two different implementations
> could save us from braking each others tests and this could be a big gain, 
> WDYT?
> (On the other hand, we'd loose updates and fixes from the other team.)
> 
> I hope one day LLDB would be able to switch to ASTImporterLookupTable from 
> noload_lookup.

Done here: https://reviews.llvm.org/D92848



Comment at: clang/unittests/AST/ASTImporterTest.cpp:5932
+typedef T U;
+A(U, T);
+  };

Second parameter seems not relevant for the test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92209

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

[PATCH] D92661: [RFC] Fix TLS and Coroutine

[Xun Li via Phabricator via cfe-commits]

lxfind added inline comments.



Comment at: llvm/include/llvm/IR/Intrinsics.td:1309
+// Intrinsic to obtain the address of a thread_local variable.
+def int_threadlocal : Intrinsic<[llvm_ptr_ty], [llvm_ptr_ty]>;
+

hoy wrote:
> lxfind wrote:
> > hoy wrote:
> > > lxfind wrote:
> > > > hoy wrote:
> > > > > hoy wrote:
> > > > > > With the intrinsic, can TLS variable reference in the same 
> > > > > > coroutine or regular routine be DCE-ed anymore?
> > > > > Sorry, I meant CSE-ed.
> > > > Since the intrinsics does not have readnone attribute, it won't be 
> > > > CSE-ed before CoroSplit.
> > > > However after CoroSplit, it will be lowered back to the direct 
> > > > reference of the TLS, and will be CSE-ed by latter passes.
> > > > I can add a test function to demonstrate that too.
> > > Sounds good. Can you please point out what optimization passes CSE-ed tls 
> > > reference without this implementation? I'm wondering if those 
> > > optimizations can be postponed to after CoroSplit. 
> > To clarify, it wasn't just CSE that would merge the references of the same 
> > TLS.
> > For instance, without this patch, a reference to "tls_variable" will just 
> > be "@tls_variable". For code like this:
> > 
> >   @tls_variable = internal thread_local global i32 0, align 4
> > 
> >   define i32* @foo(){
> > ret i32* @tls_variable
> >   }
> >   
> >   define void @bar() {
> > %tls1 = call i32* @foo()
> > ..coro.suspend..
> > %tls2 = call i32* @foo()
> > %cond = icmp eq i32* %tls1, %tls2
> >   }
> > 
> > When foo() is inlined into bar(), all uses of %tls1 will be replaced with 
> > @tls_variable.
> Thanks for the explanation. I have a dumb question. Why isn't corosplit 
> placed at the very beginning of the pipeline?
The coroutine frame size is determined during CoroSplit. So if CoroSplit 
happens too early without any optimizations, the frame size will always be very 
big and there is no chance to optimize it.
This is indeed a fundamental trade-off. If CoroSplit happens much earlier then 
it will be immune to this kind of problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92661

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

[PATCH] D92024: [clang] Implement P0692R1 from C++20 (access checking on specializations and instantiations)

[Alex Orlov via Phabricator via cfe-commits]

aorlov added inline comments.



Comment at: clang/lib/Parse/ParseTemplate.cpp:172
+
+  // TODO. This can produce wrong detection in case of a later class
+  // declaration. Example:

Quuxplusone wrote:
> I don't know the purpose of this code, but this //seems// like a super 
> important TODO.
> 
> The comment "look ahead until `{`" is also scary because
> ```
> template struct A {};
> template struct A {};
> ```
> is also a partial specialization. Why do we need to know `isSpecialization`, 
> who's //supposed// to compute it, and why does their answer need to be 
> fixed-up right here?
> 
> And is this specific piece of the patch perhaps severable into a separate 
> review? Again, I don't know this code, but... It seems like you've got one 
> part of the patch — "add a `SuppressAccessGuard` around the call to 
> `ParseDeclarator`" — which is either a 100% correct or 100% incorrect 
> implementation of P0692R1. And then you've got this other piece, a parser 
> hack, which looks much more heuristic and incomplete in nature, and looks 
> orthogonal to P0692R1.
> 
> Btw, I'm not an approver on this (and shouldn't be); if you want better 
> review you might want to ping someone who's touched this code lately 
> (according to `git blame`).
@Quuxplusone
> I don't know the purpose of this code, but this seems like a super important 
> TODO.
This is a deprecated hint, I'll remove it.
> The comment "look ahead until {" is also scary 
Nice case! I'll improve the patch.
> Why do we need to know isSpecialization
This flag helps to turn off usual access rules further:
```
Parser::ParseClassSpecifier(... const ParsedTemplateInfo &TemplateInfo, ...) {
...
  bool shouldDelayDiagsInTag =
(TemplateInfo.Kind == ParsedTemplateInfo::ExplicitInstantiation ||
 TemplateInfo.Kind == ParsedTemplateInfo::ExplicitSpecialization);
  SuppressAccessChecks diagsFromTag(*this, shouldDelayDiagsInTag);
...
}

```
> And then you've got this other piece, a parser hack,
Totally agree. It also looks like a hack for me. The point is that the 
principle of parser's workflow is that it analyzes tokens and then consume them 
one by one at the same time. So we are not confident whether we are parsing a 
primary template or specialization. Thus `TemplateInfo.Kind` is not always 
correct. To define a specialization we should look forward at the whole 
declaraion without consuming any tokens to leave them for further parsing but 
set a correct 'Kind'.
On the other side we can somehow store the context of we are parsing a template 
declaration and throw it through a bunch of calls. But I found this way much 
more complicated, than of looking at some next tokens. BTW @rsmith suggested 
this approach in the comment https://reviews.llvm.org/D78404#inline-717620
> Btw, I'm not an approver on this (and shouldn't be); if you want better 
> review you might want to ping someone who's touched this code lately 
> (according to git blame).
Thank you. I've already added all related people in the list of reviewers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92024

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

[PATCH] D92788: [clangd] NFC: Use SmallVector where possible

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 310232.
kbobyrev marked 4 inline comments as done.
kbobyrev added a comment.

Resolve review comments.

@sammccall, thanks for clarifying the cases you thought should be remaining as
they are!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92788

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/CompileCommands.cpp
  clang-tools-extra/clangd/CompileCommands.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigProvider.cpp
  clang-tools-extra/clangd/FileDistance.cpp
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/Headers.h
  clang-tools-extra/clangd/QueryDriverDatabase.cpp
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/Selection.h
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TidyProvider.cpp
  clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
  clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
  clang-tools-extra/clangd/support/Markup.cpp
  clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
  clang-tools-extra/clangd/unittests/TestIndex.cpp
  clang-tools-extra/clangd/unittests/TestTU.cpp
  clang-tools-extra/clangd/unittests/support/TraceTests.cpp

Index: clang-tools-extra/clangd/unittests/support/TraceTests.cpp
===
--- clang-tools-extra/clangd/unittests/support/TraceTests.cpp
+++ clang-tools-extra/clangd/unittests/support/TraceTests.cpp
@@ -154,7 +154,7 @@
   std::vector outputLines() {
 // Deliberately don't flush output stream, the tracer should do that.
 // This is important when clangd crashes.
-llvm::SmallVector Lines;
+llvm::SmallVector Lines;
 llvm::StringRef(Output).split(Lines, "\r\n");
 return {Lines.begin(), Lines.end()};
   }
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -187,7 +187,7 @@
 }
 
 const NamedDecl &findDecl(ParsedAST &AST, llvm::StringRef QName) {
-  llvm::SmallVector Components;
+  llvm::SmallVector Components;
   QName.split(Components, "::");
 
   auto &Ctx = AST.getASTContext();
Index: clang-tools-extra/clangd/unittests/TestIndex.cpp
===
--- clang-tools-extra/clangd/unittests/TestIndex.cpp
+++ clang-tools-extra/clangd/unittests/TestIndex.cpp
@@ -29,7 +29,7 @@
 
 static std::string replace(llvm::StringRef Haystack, llvm::StringRef Needle,
llvm::StringRef Repl) {
-  llvm::SmallVector Parts;
+  llvm::SmallVector Parts;
   Haystack.split(Parts, Needle);
   return llvm::join(Parts, Repl);
 }
Index: clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
===
--- clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
+++ clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
@@ -208,7 +208,7 @@
 }
 
 static std::string strip(llvm::StringRef Arg, llvm::StringRef Argv) {
-  llvm::SmallVector Parts;
+  llvm::SmallVector Parts;
   llvm::SplitString(Argv, Parts);
   std::vector Args = {Parts.begin(), Parts.end()};
   ArgStripper S;
Index: clang-tools-extra/clangd/support/Markup.cpp
===
--- clang-tools-extra/clangd/support/Markup.cpp
+++ clang-tools-extra/clangd/support/Markup.cpp
@@ -215,7 +215,7 @@
 
 // Trims the input and concatenates whitespace blocks into a single ` `.
 std::string canonicalizeSpaces(llvm::StringRef Input) {
-  llvm::SmallVector Words;
+  llvm::SmallVector Words;
   llvm::SplitString(Input, Words);
   return llvm::join(Words, " ");
 }
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
@@ -202,7 +202,7 @@
 struct ParsedBinaryOperator {
   BinaryOperatorKind Kind;
   SourceLocation ExprLoc;
-  llvm::SmallVector SelectedOperands;
+  llvm::SmallVector SelectedOperands;
 
   // If N is a binary operator, populate this and return true.
   bool parse(const SelectionTree::Node &N) {
Index: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
@@ -480,7 +480,7 @@
 const tooling::Replacement DeleteFuncBody(SM, DefRange->getBegin(),
   S

[PATCH] D92853: Simplifying memory globalization from the front end to move optimizations to the middle end.Memory globalization was fully implemented in the front end. There are three runtimefunction

[Jose Manuel Monsalve Diaz via Phabricator via cfe-commits]

josemonsalve2 created this revision.
josemonsalve2 requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: llvm-commits, openmp-commits, cfe-commits, sstefan1.
Herald added projects: clang, OpenMP, LLVM.

...__kmpc_data_sharing_coalesced_push_stack* __kmpc_data_sharing_pop_stackThe 
front end performed a scape analysis and created a record declare with all the 
stackvariables. Then, based on the context (isTTD and other parameters) it 
would create a pushfor the size of the record, or for that size multiplied by 
the WARP (to globalize for thewhole WARP.This PR removes the record creation, 
and it simplifies the front end to be a simple runtimecall that will be later 
on optimized in the middle end. The middle end will be able todetermine the 
stack variables that do scape, and those that do not, as well as theapprorpiate 
merging of different globalized variablesDifferential Revision: 
https://reviews.llvm.org/D90670


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92853

Files:
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
  clang/test/OpenMP/nvptx_data_sharing.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  openmp/libomptarget/deviceRTLs/common/src/data_sharing.cu
  openmp/libomptarget/deviceRTLs/interface.h

Index: openmp/libomptarget/deviceRTLs/interface.h
===
--- openmp/libomptarget/deviceRTLs/interface.h
+++ openmp/libomptarget/deviceRTLs/interface.h
@@ -432,7 +432,7 @@
 EXTERN void __kmpc_data_sharing_init_stack_spmd();
 EXTERN void *__kmpc_data_sharing_coalesced_push_stack(size_t size,
 int16_t UseSharedMemory);
-EXTERN void *__kmpc_data_sharing_push_stack(size_t size, int16_t UseSharedMemory);
+EXTERN void *__kmpc_data_sharing_push_stack(size_t size);
 EXTERN void __kmpc_data_sharing_pop_stack(void *a);
 EXTERN void __kmpc_begin_sharing_variables(void ***GlobalArgs, size_t nArgs);
 EXTERN void __kmpc_end_sharing_variables();
Index: openmp/libomptarget/deviceRTLs/common/src/data_sharing.cu
===
--- openmp/libomptarget/deviceRTLs/common/src/data_sharing.cu
+++ openmp/libomptarget/deviceRTLs/common/src/data_sharing.cu
@@ -144,11 +144,7 @@
 // the list of references to shared variables and to pre-allocate global storage
 // for holding the globalized variables.
 //
-// By default the globalized variables are stored in global memory. If the
-// UseSharedMemory is set to true, the runtime will attempt to use shared memory
-// as long as the size requested fits the pre-allocated size.
-EXTERN void *__kmpc_data_sharing_push_stack(size_t DataSize,
-int16_t UseSharedMemory) {
+EXTERN void *__kmpc_data_sharing_push_stack(size_t DataSize) {
   // Compute the total memory footprint of the requested data.
   // The master thread requires a stack only for itself. A worker
   // thread (which at this point is a warp master) will require
Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -543,7 +543,7 @@
 __OMP_RTL(__kmpc_data_sharing_init_stack_spmd, false, Void, )
 
 __OMP_RTL(__kmpc_data_sharing_coalesced_push_stack, false, VoidPtr, SizeTy, Int16)
-__OMP_RTL(__kmpc_data_sharing_push_stack, false, VoidPtr, SizeTy, Int16)
+__OMP_RTL(__kmpc_data_sharing_push_stack, false, VoidPtr, SizeTy)
 __OMP_RTL(__kmpc_data_sharing_pop_stack, false, Void, VoidPtr)
 __OMP_RTL(__kmpc_begin_sharing_variables, false, Void, VoidPtrPtrPtr, SizeTy)
 __OMP_RTL(__kmpc_end_sharing_variables, false, Void, )
Index: clang/test/OpenMP/nvptx_data_sharing.cpp
===
--- clang/test/OpenMP/nvptx_data_sharing.cpp
+++ clang/test/OpenMP/nvptx_data_sharing.cpp
@@ -2,8 +2,7 @@
 ///==///
 
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
-// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - -disable-llvm-optzns | FileCheck %s --check-prefix CK1 --check-prefix SEQ
-// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - -disable-llvm-optzns -fopenmp-cuda-parallel-target-regions | FileCheck %s --check-prefix CK1 --check-prefix PAR
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-

[PATCH] D92854: [flang][driver] Add support for `-fsyntax-only`

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski created this revision.
Herald added subscribers: dang, mgorny.
Herald added a reviewer: sscalpone.
awarzynski requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92854

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Frontend/CompilerInstance.h
  flang/include/flang/Frontend/FrontendActions.h
  flang/include/flang/Frontend/FrontendOptions.h
  flang/lib/Frontend/CMakeLists.txt
  flang/lib/Frontend/CompilerInstance.cpp
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Flang-Driver/syntax-only.f90
  flang/unittests/Frontend/PrintPreprocessedTest.cpp

Index: flang/unittests/Frontend/PrintPreprocessedTest.cpp
===
--- flang/unittests/Frontend/PrintPreprocessedTest.cpp
+++ flang/unittests/Frontend/PrintPreprocessedTest.cpp
@@ -76,4 +76,60 @@
   llvm::sys::fs::remove(inputFile);
   compInst.ClearOutputFiles(/*EraseFiles=*/true);
 }
+
+TEST(FrontendAction, ParseSyntaxOnly) {
+  std::string inputFile = "test-file.f";
+  std::error_code ec;
+
+  // 1. Create the input file for the file manager
+  // AllSources (which is used to manage files inside every compiler instance),
+  // works with paths. This means that it requires a physical file. Create one.
+  std::unique_ptr os{
+  new llvm::raw_fd_ostream(inputFile, ec, llvm::sys::fs::OF_None)};
+  if (ec)
+FAIL() << "Fail to create the file need by the test";
+
+  // Populate the input file with the pre-defined input and flush it.
+  *(os) << "! if_stmt.f90:\n"
+<< "IF (A > 0.0) IF (B < 0.0) A = LOG (A)\n"
+<< "END";
+  os.reset();
+
+  // Get the path of the input file
+  llvm::SmallString<64> cwd;
+  if (std::error_code ec = llvm::sys::fs::current_path(cwd))
+FAIL() << "Failed to obtain the current working directory";
+  std::string testFilePath(cwd.c_str());
+  testFilePath += "/" + inputFile;
+
+  // 2. Prepare the compiler (CompilerInvocation + CompilerInstance)
+  CompilerInstance compInst;
+  compInst.CreateDiagnostics();
+  auto invocation = std::make_shared();
+  invocation->frontendOpts().programAction_ = ParseSyntaxOnly;
+
+  compInst.set_invocation(std::move(invocation));
+  compInst.frontendOpts().inputs_.push_back(
+  FrontendInputFile(testFilePath, Language::Fortran));
+
+  // 3. Set-up the output stream for the semantic diagnostics.
+  llvm::SmallVector outputDiagBuffer;
+  std::unique_ptr outputStream(
+  new llvm::raw_svector_ostream(outputDiagBuffer));
+  compInst.set_semaOutputStream(std::move(outputStream));
+
+  // 4. Executre the ParseSyntaxOnly action
+  bool success = ExecuteCompilerInvocation(&compInst);
+
+  // 5. Validate the expected output
+  EXPECT_TRUE(success);
+  EXPECT_TRUE(!outputDiagBuffer.empty());
+  EXPECT_TRUE(
+  llvm::StringRef(outputDiagBuffer.data())
+  .startswith(
+  ":2:14: error: IF statement is not allowed in IF statement\n"));
+
+  // 6. Clear the input files.
+  llvm::sys::fs::remove(inputFile);
+}
 } // namespace
Index: flang/test/Flang-Driver/syntax-only.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/syntax-only.f90
@@ -0,0 +1,5 @@
+! RUN: %flang-new -fc1 -fsyntax-only %s 2>&1 | FileCheck %s
+
+! CHECK: IF statement is not allowed in IF statement
+IF (A > 0.0) IF (B < 0.0) A = LOG (A)
+END
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -32,6 +32,9 @@
   case PrintPreprocessedInput:
 return std::make_unique();
 break;
+  case ParseSyntaxOnly:
+return std::make_unique();
+break;
   default:
 break;
 // TODO:
Index: flang/lib/Frontend/FrontendActions.cpp
===
--- flang/lib/Frontend/FrontendActions.cpp
+++ flang/lib/Frontend/FrontendActions.cpp
@@ -11,6 +11,7 @@
 #include "flang/Parser/parsing.h"
 #include "flang/Parser/provenance.h"
 #include "flang/Parser/source.h"
+#include "flang/Semantics/semantics.h"
 
 using namespace Fortran::frontend;
 
@@ -68,3 +69,28 @@
 return;
   }
 }
+
+void SyntaxOnlyAction::ExecuteAction() {
+  CompilerInstance &ci = this->instance();
+
+  // TODO: These should be specifiable by users. For now just use the defaults.
+  common::LanguageFeatureControl features;
+  Fortran::common::IntrinsicTypeDefaultKinds defaultKinds;
+
+  // Parse
+  ci.parsing().Parse(llvm::outs());
+  auto &parseTree{*ci.parsing().parseTree()};
+
+  // Prepare semantincs
+  Fortran::semantics::SemanticsContext semanticsContext{
+  defaultKinds, features,

[PATCH] D92775: [clang][cli] Add flexible TableGen multiclass for boolean options

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese added inline comments.



Comment at: clang/unittests/Frontend/CompilerInvocationTest.cpp:226
   ASSERT_THAT(GeneratedArgs,
-  Contains(StrEq("-fno-experimental-new-pass-manager")));
+  Not(Contains(StrEq("-fno-experimental-new-pass-manager";
   ASSERT_THAT(GeneratedArgs,

dexonsmith wrote:
> jansvoboda11 wrote:
> > dexonsmith wrote:
> > > Can you clarify why this was dropped? Was it previously emitted due to a 
> > > limitation in the implementation, or are we no longer supporting options 
> > > that always get emitted for clarity?
> > This option was the only one using the old infrastructure 
> > (`BooleanMarshalledFFlag`).
> > It was set up to always generate the flag, even when it wasn't necessary 
> > (this flag sets the keypath to its default value).
> > 
> > I think we should aim to generate only command line arguments that are 
> > necessary to preserve the original invocation semantics.
> > I imagine this will be useful when debugging: one won't need to scan 
> > hundreds of flags that don't have any effect on CompilerInvocation.
> This is a change in direction; the original thinking was that some options 
> should always be emitted for human readability. I don’t feel too strongly 
> about it, but I think this should be changed / dropped independently of other 
> work if it’s done. I suggest splitting this out; I’d also like to hear 
> @Bigcheese’s thoughts on that change since he did more of the original 
> reviews. 
In general there are some options that should always be kept, such as the 
triple, but I don't see any reason to always keep 
`-fno-experimental-new-pass-manager`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92775

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