[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

2022-09-04 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

IMHO the check looks good, but I do have concerns about correctness of the 
transformation in specific cases, especially overloaded operators.

If the conditions are inverted, it might be the case that the inverted operator 
is not overloaded, resulting in compilation errors.
I think that should be guarded against.

And then there is the more subtle issue of different semantics of the operators.

Hypothetical Matrix Algebra Situation:

  // A, B are matrices of booleans with identical dimensions
  // A && B will do '&&' on each matrix element
  // The matrices overload 'operator bool()' for implicit conversion to 'bool',
  // true := any element != false
  // false := all false
  if (A && B) {
// !C inverts each boolean in C, same '&&' application
if (B && !C) {
  padLines();
}
  }

Transformed to

  if (!A )
continue;
  if ( !B)
continue;
  if (!B )
continue;
  if ( C)
continue;
  padLines();

is highly likely not a correct and equivalent transformation.

My point is not so much about the concrete example, but more general.
C++ allows to implement operators with different semantics and classes must not 
behave like 'int' for the operators. Such overloads are not necessarily 
incorrect. E.g. `boost::sml` uses the operator overloading to define state 
machines, which does not follow anything close to the semantics we need for 
this check.
Expression-template libraries (especially linear algebra and so on) might 
either break or change meaning as well.

In my personal opinion the check should at least have an option to disable 
transformations for classes with overloaded operators and verify that the 
transformation would still compile if done anyway.
I think even better would be a "concept check" to at least verify that the type 
in question models a boolean with its operators. But I am not sure how far such 
a check should go and I am aware that it would not be perfect anyway.




Comment at: clang-tools-extra/clang-tidy/readability/UseEarlyExitsCheck.cpp:329
+assert(BSize >= CheckAlias.size() + OptName.size());
+memcpy(Buff, CheckAlias.data(), CheckAlias.size());
+memcpy(Buff + CheckAlias.size(), OptName.data(), OptName.size());

i would really prefer to just use strings here.
`std::string Buff = CheckAlias.str() + OptName.str();` is much easier to 
understand and equivalent (?)
 
this function is called only once in the constructor as well, so speed and 
allocations are not valid in my opinion.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability/use-early-exits.rst:63
+void Process(bool A, bool B) {
+  if (A && B) {
+// Long processing.

njames93 wrote:
> JonasToth wrote:
> > njames93 wrote:
> > > JonasToth wrote:
> > > > if this option is false, the transformation would be `if(!(A && B))`, 
> > > > right?
> > > > 
> > > > should demorgan rules be applied or at least be mentioned here? I think 
> > > > transforming to `if (!A || !B)` is at least a viable option for enough 
> > > > users.
> > > Once this is in, I plan to merge some common code with the 
> > > simplify-boolean-expr logic for things like demorgan processing. Right 
> > > now the transformation happens, the simplify boolean suggests a demorgan 
> > > transformation of you run the output through clang tidy.
> > a short reference to the `readability-simplify-boolean-expr` check in the 
> > user facing docs would be great.
> > i personally find it ok if the users "pipe" multiple checks together to 
> > reach a final transformation.
> > 
> > would this check then use the same settings as the 
> > `readability-simplify-boolean-expr` check? (that of course off topic and 
> > does not relate to this patch :) )
> I'm not sure it's really necessary to mention that the fix would likely need 
> another fix, besides that comment would just be removed in the follow up.
ok, thats good with me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130181

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


[PATCH] D133092: [clang] fix generation of .debug_aranges with LTO

2022-09-04 Thread Azat Khuzhin via Phabricator via cfe-commits
azat updated this revision to Diff 457850.
azat added a comment.

Adjust the test (to fix build on windows)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133092

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/debug-options.c


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -246,7 +246,11 @@
 // RUN: %clang -### -c -glldb %s 2>&1 | FileCheck -check-prefix=NOPUB %s
 // RUN: %clang -### -c -glldb -gno-pubnames %s 2>&1 | FileCheck 
-check-prefix=NOPUB %s
 //
-// RUN: %clang -### -c -gdwarf-aranges %s 2>&1 | FileCheck 
-check-prefix=GARANGE %s
+// RUN: %clang -### -target x86_64-unknown-linux -c -gdwarf-aranges %s 2>&1 | 
FileCheck -check-prefix=GARANGE %s
+// RUN: %clang -### -target x86_64-unknown-linux -flto -gdwarf-aranges %s 2>&1 
| FileCheck -check-prefix=LDGARANGE %s
+// RUN: %clang -### -target x86_64-unknown-linux -flto=thin -gdwarf-aranges %s 
2>&1 | FileCheck -check-prefix=LDGARANGE %s
+// RUN: %clang -### -target x86_64-unknown-linux -fuse-ld=lld -flto 
-gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=LLDGARANGE %s
+// RUN: %clang -### -target x86_64-unknown-linux -fuse-ld=lld -flto=thin 
-gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=LLDGARANGE %s
 //
 // RUN: %clang -### -fdebug-types-section -target x86_64-unknown-linux %s 2>&1 
\
 // RUN:| FileCheck -check-prefix=FDTS %s
@@ -371,6 +375,8 @@
 // NORNGBSE-NOT: -fdebug-ranges-base-address
 //
 // GARANGE-DAG: -generate-arange-section
+// LDGARANGE-NOT: {{".*lld.*"}} {{.*}} "-generate-arange-section"
+// LLDGARANGE-DAG: {{".*lld.*"}} {{.*}} "-generate-arange-section"
 //
 // FDTS: "-mllvm" "-generate-type-units"
 // FDTSE: error: unsupported option '-fdebug-types-section' for target 
'x86_64-apple-darwin'
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -506,6 +506,14 @@
 Suffix,
 Plugin);
 CmdArgs.push_back(Args.MakeArgString(Plugin));
+  } else {
+// NOTE: That it is not possible to use lld for PS4, hence no need to
+// handle SCE (like in Clang.cpp::renderDebugOptions())
+bool NeedAranges = Args.hasArg(options::OPT_gdwarf_aranges);
+if (NeedAranges) {
+  CmdArgs.push_back(Args.MakeArgString("-mllvm"));
+  CmdArgs.push_back(Args.MakeArgString("-generate-arange-section"));
+}
   }
 
   // Try to pass driver level flags relevant to LTO code generation down to


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -246,7 +246,11 @@
 // RUN: %clang -### -c -glldb %s 2>&1 | FileCheck -check-prefix=NOPUB %s
 // RUN: %clang -### -c -glldb -gno-pubnames %s 2>&1 | FileCheck -check-prefix=NOPUB %s
 //
-// RUN: %clang -### -c -gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=GARANGE %s
+// RUN: %clang -### -target x86_64-unknown-linux -c -gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=GARANGE %s
+// RUN: %clang -### -target x86_64-unknown-linux -flto -gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=LDGARANGE %s
+// RUN: %clang -### -target x86_64-unknown-linux -flto=thin -gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=LDGARANGE %s
+// RUN: %clang -### -target x86_64-unknown-linux -fuse-ld=lld -flto -gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=LLDGARANGE %s
+// RUN: %clang -### -target x86_64-unknown-linux -fuse-ld=lld -flto=thin -gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=LLDGARANGE %s
 //
 // RUN: %clang -### -fdebug-types-section -target x86_64-unknown-linux %s 2>&1 \
 // RUN:| FileCheck -check-prefix=FDTS %s
@@ -371,6 +375,8 @@
 // NORNGBSE-NOT: -fdebug-ranges-base-address
 //
 // GARANGE-DAG: -generate-arange-section
+// LDGARANGE-NOT: {{".*lld.*"}} {{.*}} "-generate-arange-section"
+// LLDGARANGE-DAG: {{".*lld.*"}} {{.*}} "-generate-arange-section"
 //
 // FDTS: "-mllvm" "-generate-type-units"
 // FDTSE: error: unsupported option '-fdebug-types-section' for target 'x86_64-apple-darwin'
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -506,6 +506,14 @@
 Suffix,
 Plugin);
 CmdArgs.push_back(Args.MakeArgString(Plugin));
+  } else {
+// NOTE: That it is not possible to use lld for PS4, hence no need to
+// handle SCE (like in Clang.cpp::renderDebugOptions())
+bool NeedAranges = Args.hasArg(options::OPT_gdwarf_aranges);
+if (NeedAranges) {
+  CmdArgs.push_back(Args.MakeArgString("-mllvm"));
+  CmdArgs.push_back(Args.M

Re: [clang] b7a7aee - [clang] Qualify auto in range-based for loops (NFC)

2022-09-04 Thread Aaron Ballman via cfe-commits
FWIW, sweeping NFC changes like this cause a fair amount of code churn
(which makes tools like git blame a bit harder to use) for a
relatively small improvement to code readability, which is why our
developer policy asks that you "Avoid committing formatting- or
whitespace-only changes outside of code you plan to make subsequent
changes to." In the future, I'd caution against doing such large-scale
sweeping NFC changes outside of areas you're actively working on
unless there's some wider discussion with the community first. That
said, all of your changes are all improvements, so thank you for them!

Some of the changes you made would have ideally made it more clear
when the deduced type is a pointer to a const object instead of hiding
the qualifier behind the deduction. I've pointed out a couple such
places below, but don't feel obligated to go back and change anything
unless you're going to be touching other code in the area.

~Aaron


On Sun, Sep 4, 2022 at 2:27 AM Kazu Hirata via cfe-commits
 wrote:
>
>
> Author: Kazu Hirata
> Date: 2022-09-03T23:27:27-07:00
> New Revision: b7a7aeee90cffefd0f73b8d9f44ab4d1d5123d05
>
> URL: 
> https://github.com/llvm/llvm-project/commit/b7a7aeee90cffefd0f73b8d9f44ab4d1d5123d05
> DIFF: 
> https://github.com/llvm/llvm-project/commit/b7a7aeee90cffefd0f73b8d9f44ab4d1d5123d05.diff
>
> LOG: [clang] Qualify auto in range-based for loops (NFC)
>
> Added:
>
>
> Modified:
> clang/lib/ARCMigrate/ObjCMT.cpp
> clang/lib/ARCMigrate/TransGCAttrs.cpp
> clang/lib/AST/ASTContext.cpp
> clang/lib/AST/ASTImporter.cpp
> clang/lib/AST/Decl.cpp
> clang/lib/AST/DeclObjC.cpp
> clang/lib/AST/ODRHash.cpp
> clang/lib/AST/OpenMPClause.cpp
> clang/lib/AST/StmtProfile.cpp
> clang/lib/AST/Type.cpp
> clang/lib/Analysis/CFG.cpp
> clang/lib/Analysis/ThreadSafetyCommon.cpp
> clang/lib/CodeGen/CGBlocks.cpp
> clang/lib/CodeGen/CGCall.cpp
> clang/lib/CodeGen/CGClass.cpp
> clang/lib/CodeGen/CGDebugInfo.cpp
> clang/lib/CodeGen/CGDeclCXX.cpp
> clang/lib/CodeGen/CGExpr.cpp
> clang/lib/CodeGen/CGObjCGNU.cpp
> clang/lib/CodeGen/CGObjCMac.cpp
> clang/lib/CodeGen/CodeGenFunction.cpp
> clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
> clang/lib/CodeGen/SwiftCallingConv.cpp
> clang/lib/Driver/Compilation.cpp
> clang/lib/Driver/Driver.cpp
> clang/lib/Driver/ToolChains/Clang.cpp
> clang/lib/Driver/ToolChains/CommonArgs.cpp
> clang/lib/Driver/ToolChains/Gnu.cpp
> clang/lib/Driver/ToolChains/HIPAMD.cpp
> clang/lib/Format/Format.cpp
> clang/lib/Frontend/FrontendActions.cpp
> clang/lib/Index/USRGeneration.cpp
> clang/lib/Sema/IdentifierResolver.cpp
> clang/lib/Sema/Sema.cpp
> clang/lib/Sema/SemaCodeComplete.cpp
> clang/lib/Sema/SemaDecl.cpp
> clang/lib/Sema/SemaDeclAttr.cpp
> clang/lib/Sema/SemaDeclCXX.cpp
> clang/lib/Sema/SemaDeclObjC.cpp
> clang/lib/Sema/SemaExpr.cpp
> clang/lib/Sema/SemaExprCXX.cpp
> clang/lib/Sema/SemaInit.cpp
> clang/lib/Sema/SemaLambda.cpp
> clang/lib/Sema/SemaLookup.cpp
> clang/lib/Sema/SemaObjCProperty.cpp
> clang/lib/Sema/SemaOpenMP.cpp
> clang/lib/Sema/SemaOverload.cpp
> clang/lib/Sema/SemaTemplateDeduction.cpp
> clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
> clang/lib/Serialization/ASTReader.cpp
> clang/lib/Serialization/ASTWriterDecl.cpp
> clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
> clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
> clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
> clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp
> clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp
> clang/lib/StaticAnalyzer/Core/CallEvent.cpp
> clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
> clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
> clang/lib/Tooling/Tooling.cpp
>
> Removed:
>
>
>
> 
> diff  --git a/clang/lib/ARCMigrate/ObjCMT.cpp 
> b/clang/lib/ARCMigrate/ObjCMT.cpp
> index 26f751b77f86a..fe0ce4c5cdc3a 100644
> --- a/clang/lib/ARCMigrate/ObjCMT.cpp
> +++ b/clang/lib/ARCMigrate/ObjCMT.cpp
> @@ -792,7 +792,7 @@ static bool UseNSOptionsMacro(Preprocessor &PP, 
> ASTContext &Ctx,
>bool PowerOfTwo = true;
>bool AllHexdecimalEnumerator = true;
>uint64_t MaxPowerOfTwoVal = 0;
> -  for (auto Enumerator : EnumDcl->enumerators()) {
> +  for (auto *Enumerator : EnumDcl->enumerators()) {
>  const Expr *InitExpr = Enumerator->getInitExpr();
>  if (!InitExpr) {
>PowerOfTwo = false;
>
> diff  --git a/clang/lib/ARCMigrate/TransGCAttrs.cpp 
> b/clang/lib/ARCMigrate/TransGCAttrs.cpp
> index 99a61e0842a76..b94aee2de573e 100644
> --- a/clang/lib/ARCMigrate/TransGCAttrs.cpp
> +++ b/clang/lib/ARCMigrate/TransGCAttrs.cpp
> @@ -158,7 +158,7 @@ class GCAttrsCollector : public 
> RecursiveASTVisitor {
>  if (!D)
>

[PATCH] D133088: [Clang] Fix wrong diagnostic for scope identifier with internal linkage

2022-09-04 Thread Jun Zhang via Phabricator via cfe-commits
junaire updated this revision to Diff 457855.
junaire added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133088

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Parser/cxx1z-decomposition.cpp
  clang/test/Sema/array-init.c
  clang/test/Sema/err-decl-block-extern-no-init.c
  clang/test/Sema/private-extern.c

Index: clang/test/Sema/private-extern.c
===
--- clang/test/Sema/private-extern.c
+++ clang/test/Sema/private-extern.c
@@ -69,9 +69,9 @@
 struct s0 { int x; };
 
 void f9(void) {
-  extern int g15 = 0; // expected-error{{'extern' variable cannot have an initializer}}
+  extern int g15 = 0; // expected-error{{declaration of block scope identifier with external linkage shall have no initializer}}
   // FIXME: linkage specifier in warning.
-  __private_extern__ int g16 = 0; // expected-error{{'extern' variable cannot have an initializer}}
+  __private_extern__ int g16 = 0; // expected-error{{declaration of block scope identifier with external linkage shall have no initializer}}
 }
 
 extern int g17;
Index: clang/test/Sema/err-decl-block-extern-no-init.c
===
--- /dev/null
+++ clang/test/Sema/err-decl-block-extern-no-init.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify %s
+static int x;
+
+void foo(void)
+{
+extern int x = 1; // expected-error {{declaration of block scope identifier with internal linkage shall have no initializer}}
+}
+
+int y;
+
+void bar(void)
+{
+extern int y = 1; // expected-error {{declaration of block scope identifier with external linkage shall have no initializer}}
+
+}
Index: clang/test/Sema/array-init.c
===
--- clang/test/Sema/array-init.c
+++ clang/test/Sema/array-init.c
@@ -48,7 +48,7 @@
 
   struct threeElements *p = 7; // expected-error{{incompatible integer to pointer conversion initializing 'struct threeElements *' with an expression of type 'int'}}
 
-  extern int blockScopeExtern[3] = { 1, 3, 5 }; // expected-error{{'extern' variable cannot have an initializer}}
+  extern int blockScopeExtern[3] = { 1, 3, 5 }; // expected-error{{declaration of block scope identifier with external linkage shall have no initializer}}
 
   static long x2[3] = { 1.0,
 "abc", // expected-error{{incompatible pointer to integer conversion initializing 'long' with an expression of type 'char[4]'}}
Index: clang/test/Parser/cxx1z-decomposition.cpp
===
--- clang/test/Parser/cxx1z-decomposition.cpp
+++ clang/test/Parser/cxx1z-decomposition.cpp
@@ -69,7 +69,7 @@
 // storage-class-specifiers
 static auto &[a] = n; // expected-warning {{declared 'static' is a C++20 extension}}
 thread_local auto &[b] = n; // expected-warning {{declared 'thread_local' is a C++20 extension}}
-extern auto &[c] = n; // expected-error {{cannot be declared 'extern'}} expected-error {{cannot have an initializer}}
+extern auto &[c] = n; // expected-error {{cannot be declared 'extern'}} expected-error {{declaration of block scope identifier with external linkage shall have no initializer}}
 struct S {
   mutable auto &[d] = n; // expected-error {{not permitted in this context}}
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12771,9 +12771,14 @@
 return;
   }
 
+  // C99 6.7.8p5. C++ has no such restriction, but that is a defect.
   if (VDecl->isLocalVarDecl() && VDecl->hasExternalStorage()) {
-// C99 6.7.8p5. C++ has no such restriction, but that is a defect.
-Diag(VDecl->getLocation(), diag::err_block_extern_cant_init);
+unsigned LinkageKind = /*external*/ 0;
+// C2x 6.7.10p6.
+if (VDecl->getFormalLinkage() == InternalLinkage)
+  LinkageKind = /*internal*/ 1;
+
+Diag(VDecl->getLocation(), diag::err_block_extern_cant_init) << LinkageKind;
 VDecl->setInvalidDecl();
 return;
   }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5901,7 +5901,8 @@
 : Error<"variable %0 cannot be declared both 'extern' and with the "
 "'loader_uninitialized' attribute">;
 def err_block_extern_cant_init : Error<
-  "'extern' variable cannot have an initializer">;
+  "declaration of block scope identifier with %select{external|internal}0 "
+  "linkage shall have no initializer">;
 def warn_extern_init : Warning<"'extern' variable has an initializer">,
   

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-04 Thread Yusuke Kadowaki via Phabricator via cfe-commits
yusuke-kadowaki updated this revision to Diff 457864.
yusuke-kadowaki marked 8 inline comments as done.
yusuke-kadowaki added a comment.

- Revert doc
- Revert rst as well
- Apply format
- Update implementation to deal with the setting of MaxEmptyLinesToKeep
- Add test for the combination with MaxEmptyLinesToKeep


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestComments.cpp

Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -2858,6 +2858,200 @@
"int a; //\n");
 }
 
+TEST_F(FormatTestComments, AlignTrailingCommentsAcrossEmptyLines) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignConsecutiveTrailingComments.AcrossEmptyLines = true;
+  verifyFormat("#include \"a.h\"  // simple\n"
+   "\n"
+   "#include \"aa.h\" // example case\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "\n"
+   "#include \"aa.h\"  // two empty lines\n"
+   "\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // one\n"
+   "#include \"aa.h\" // empty line\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align trailing comments\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\" // across a line without comment\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\"  // two lines without comment\n"
+   "#include \"a.h\"\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "#include \"a.h\"\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // a line without\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  // Start of testing the combination with MaxEmptyLinesToKeep
+  Style.MaxEmptyLinesToKeep = 0;
+  EXPECT_EQ("int a;   // comment\n"
+"int ab;  // comment\n"
+"int abc; // comment\n",
+format("int a; // comment\n"
+   "int ab; // comment\n"
+   "\n"
+   "int abc; // comment\n",
+   Style));
+
+  Style.MaxEmptyLinesToKeep = 1;
+  EXPECT_EQ("int a;   // comment\n"
+"int ab;  // comment\n"
+"\n"
+"int abc; // comment\n",
+format("int a; // comment\n"
+   "int ab; // comment\n"
+   "\n"
+   "\n"
+   "int abc; // comment\n",
+   Style));
+
+  Style.MaxEmptyLinesToKeep = 2;
+  EXPECT_EQ("int a;   // comment\n"
+"int ab;  // comment\n"
+"\n"
+"\n"
+"int abc; // comment\n",
+format("int a; // comment\n"
+   "int ab; // comment\n"
+   "\n"
+   "\n"
+   "\n"
+   "int abc; // comment\n",
+   Style));
+
+  // Reset the setting
+  Style.MaxEmptyLinesToKeep = 1;
+  // End of testing the combination with MaxEmptyLinesToKeep
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"int a;  // long\n"
+"// long\n"
+"\n"
+"// long",
+format("int ab; // line\n"
+   "int a; // long long\n"
+   "\n"
+   "// long",
+   Style));
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"\n"
+"int a;  // long\n"
+"// long\n",
+format("int ab; // line\n"
+   "\n"
+   "int a; // long long\n",
+   Style));
+
+  Style.ColumnLimit = 30;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int bar =\n"
+"1234;  // This is a very\n"
+"   // long comment\n"
+"   // which is wrapped\n"
+"   // arround.\n"
+"

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-04 Thread Yusuke Kadowaki via Phabricator via cfe-commits
yusuke-kadowaki added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:3698
 
-  QualifierOrder: ['inline', 'static', 'type', 'const']
+  QualifierOrder: ['inline', 'static' , 'type', 'const']
 

HazardyKnusperkeks wrote:
> Anyone knows where this comes from?
> You are using the script to generate the rst, right?
Yes, I'm using `dump_format_style.py`.



Comment at: clang/include/clang/Format/Format.h:154
   /// \code
   ///   AlignConsecutiveMacros: AcrossEmptyLines
   ///

HazardyKnusperkeks wrote:
> yusuke-kadowaki wrote:
> > HazardyKnusperkeks wrote:
> > > The change/addition has to be here, since here it directly states 
> > > `AlignConsecutiveMacros`.
> > What are you meaning to say here? I thought saying `AlignConsecutiveStyle` 
> > is used for multiple options is what we are trying to do.
> > 
> > Should we say something like, 
> > > Here AlignConsecutiveMacros is used as an example, but in practice 
> > > AlignConsecutiveStyle is also used with other options.
> > in this place?
> The example mentions explicitly only `AlignConsecutiveMacros` which is a bit 
> misleading if you are only looking at the documentation of 
> `AlignConsecutiveAssignments` for example.
> 
> This is not your fault, and I'm fine with ignoring it here. A separate patch 
> to fix that is welcome. :)
Got it. Reverted for now.



Comment at: clang/lib/Format/WhitespaceManager.cpp:930
   unsigned Newlines = 0;
+  unsigned int NewLineThreshold = 
Style.AlignConsecutiveTrailingComments.AcrossEmptyLines ? 2 : 1;
   for (unsigned i = 0, e = Changes.size(); i != e; ++i) {

HazardyKnusperkeks wrote:
> And accompanied by a short test. That should be everything to support the 
> mixture of the options, right?
> That should be everything to support the mixture of the options, right?
I think so.

Done.



Comment at: clang/lib/Format/WhitespaceManager.cpp:984
+}
+else if (BreakBeforeNext || Newlines > NewLineThreshold ||
(ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn) ||

HazardyKnusperkeks wrote:
> Is this clang-format formatted?
I forgot that. Also formatted some other diffs.



Comment at: clang/unittests/Format/FormatTestComments.cpp:4262
 /\
-/ 
+/
   )",

HazardyKnusperkeks wrote:
> Please revert this.
Reverted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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


[clang] a46154c - [analyzer] Warn if the size of the array in `new[]` is undefined

2022-09-04 Thread via cfe-commits

Author: isuckatcs
Date: 2022-09-04T23:06:58+02:00
New Revision: a46154cb1cd09aa26bc803d8696e6e9283aac6a9

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

LOG: [analyzer] Warn if the size of the array in `new[]` is undefined

This patch introduces a new checker, called NewArraySize checker,
which detects if the expression that yields the element count of
the array in new[], results in an Undefined value.

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

Added: 
clang/lib/StaticAnalyzer/Checkers/UndefinedNewArraySizeChecker.cpp
clang/test/Analysis/undefined-new-element.cpp

Modified: 
clang/docs/analyzer/checkers.rst
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/test/Analysis/Issue56873.cpp

Removed: 




diff  --git a/clang/docs/analyzer/checkers.rst 
b/clang/docs/analyzer/checkers.rst
index 623a520574e2..2903b3d46441 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -245,6 +245,22 @@ Check for uninitialized values being returned to the 
caller.
return x; // warn
  }
 
+.. _core-uninitialized-NewArraySize:
+
+core.uninitialized.NewArraySize (C++)
+"
+
+Check if the element count in new[] is garbage or undefined.
+
+.. code-block:: cpp
+
+  void test() {
+int n;
+int *arr = new int[n]; // warn: Element count in new[] is a garbage value
+delete[] arr;
+  }
+
+
 .. _cplusplus-checkers:
 
 

diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td 
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index cf8cec3b13c3..3dd2c7c1606c 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -437,6 +437,10 @@ def ReturnUndefChecker : Checker<"UndefReturn">,
   HelpText<"Check for uninitialized values being returned to the caller">,
   Documentation;
 
+def UndefinedNewArraySizeChecker : Checker<"NewArraySize">,
+  HelpText<"Check if the size of the array in a new[] expression is 
undefined">,
+  Documentation;
+
 } // end "core.uninitialized"
 
 
//===--===//

diff  --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
index 50a27a211ef0..42a51b4c8e9e 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -1033,6 +1033,18 @@ class CXXAllocatorCall : public AnyFunctionCall {
 return getOriginExpr()->getNumPlacementArgs() + getNumImplicitArgs();
   }
 
+  bool isArray() const { return getOriginExpr()->isArray(); }
+
+  Optional getArraySizeExpr() const {
+return getOriginExpr()->getArraySize();
+  }
+
+  SVal getArraySizeVal() const {
+assert(isArray() && "The allocator call doesn't allocate and array!");
+
+return getState()->getSVal(*getArraySizeExpr(), getLocationContext());
+  }
+
   const Expr *getArgExpr(unsigned Index) const override {
 // The first argument of an allocator call is the size of the allocation.
 if (Index < getNumImplicitArgs())

diff  --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt 
b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index 84886b93d2e4..c6be8fe21288 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -120,6 +120,7 @@ add_clang_library(clangStaticAnalyzerCheckers
   UndefResultChecker.cpp
   UndefinedArraySubscriptChecker.cpp
   UndefinedAssignmentChecker.cpp
+  UndefinedNewArraySizeChecker.cpp
   UninitializedObject/UninitializedObjectChecker.cpp
   UninitializedObject/UninitializedPointee.cpp
   UnixAPIChecker.cpp

diff  --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index b281d9b267e8..5613b8f7b4c9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -1733,6 +1733,10 @@ ProgramStateRef 
MallocChecker::MallocMemAux(CheckerContext &C,
   // Fill the region with the initialization value.
   State = State->bindDefaultInitial(RetVal, Init, LCtx);
 
+  // If Size is somehow undefined at this point, this line prevents a crash.
+  if (Size.isUndef())
+Size = UnknownVal();
+
   // Set the region's extent.
   State = setDynamicExtent(State, RetVal.getAsRegion(),
Size.castAs(), svalBuilder);

diff  --git 
a/clang/lib/StaticAnalyzer/Checkers/Unde

[PATCH] D131299: [analyzer] Warn if the size of the array in `new[]` is undefined

2022-09-04 Thread Domján Dániel via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa46154cb1cd0: [analyzer] Warn if the size of the array in 
`new[]` is undefined (authored by isuckatcs).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131299

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/UndefinedNewArraySizeChecker.cpp
  clang/test/Analysis/Issue56873.cpp
  clang/test/Analysis/undefined-new-element.cpp

Index: clang/test/Analysis/undefined-new-element.cpp
===
--- /dev/null
+++ clang/test/Analysis/undefined-new-element.cpp
@@ -0,0 +1,69 @@
+// RUN: %clang_analyze_cc1 %s -analyzer-checker=core.uninitialized.NewArraySize -analyzer-output=text -verify
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+void checkUndefinedElmenetCountValue() {
+  int n;
+  // expected-note@-1{{'n' declared without an initial value}}
+
+  int *arr = new int[n]; // expected-warning{{Element count in new[] is a garbage value}}
+  // expected-note@-1{{Element count in new[] is a garbage value}}
+}
+
+void checkUndefinedElmenetCountMultiDimensionalValue() {
+  int n;
+  // expected-note@-1{{'n' declared without an initial value}}
+
+  auto *arr = new int[n][5]; // expected-warning{{Element count in new[] is a garbage value}}
+  // expected-note@-1{{Element count in new[] is a garbage value}}
+}
+
+void checkUndefinedElmenetCountReference() {
+  int n;
+  // expected-note@-1{{'n' declared without an initial value}}
+
+  int &ref = n;
+  // expected-note@-1{{'ref' initialized here}}
+
+  int *arr = new int[ref]; // expected-warning{{Element count in new[] is a garbage value}}
+  // expected-note@-1{{Element count in new[] is a garbage value}}
+}
+
+void checkUndefinedElmenetCountMultiDimensionalReference() {
+  int n;
+  // expected-note@-1{{'n' declared without an initial value}}
+
+  int &ref = n;
+  // expected-note@-1{{'ref' initialized here}}
+
+  auto *arr = new int[ref][5]; // expected-warning{{Element count in new[] is a garbage value}}
+  // expected-note@-1{{Element count in new[] is a garbage value}}
+}
+
+int foo() {
+  int n;
+
+  return n;
+}
+
+void checkUndefinedElmenetCountFunction() {
+  int *arr = new int[foo()]; // expected-warning{{Element count in new[] is a garbage value}}
+  // expected-note@-1{{Element count in new[] is a garbage value}}
+}
+
+void checkUndefinedElmenetCountMultiDimensionalFunction() {
+  auto *arr = new int[foo()][5]; // expected-warning{{Element count in new[] is a garbage value}}
+  // expected-note@-1{{Element count in new[] is a garbage value}}
+}
+
+void *malloc(size_t);
+
+void checkUndefinedPlacementElementCount() {
+  int n;
+  // expected-note@-1{{'n' declared without an initial value}}
+  
+  void *buffer = malloc(sizeof(std::string) * 10);
+  std::string *p =
+  ::new (buffer) std::string[n]; // expected-warning{{Element count in new[] is a garbage value}}
+  // expected-note@-1{{Element count in new[] is a garbage value}}
+}
Index: clang/test/Analysis/Issue56873.cpp
===
--- clang/test/Analysis/Issue56873.cpp
+++ clang/test/Analysis/Issue56873.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection -verify %s
 
 void clang_analyzer_warnIfReached();
 
Index: clang/lib/StaticAnalyzer/Checkers/UndefinedNewArraySizeChecker.cpp
===
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/UndefinedNewArraySizeChecker.cpp
@@ -0,0 +1,80 @@
+//===--- UndefinedNewArraySizeChecker.cpp ---*- C++ -*--==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This defines UndefinedNewArraySizeChecker, a builtin check in ExprEngine
+// that checks if the size of the array in a new[] expression is undefined.
+//
+//===--===//
+
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticA

[PATCH] D132713: [clang-tidy] Skip union-like classes in use-equals-default

2022-09-04 Thread Alexander Shaposhnikov via Phabricator via cfe-commits
alexander-shaposhnikov added a comment.

gentle ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132713

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


[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

2022-09-04 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov added inline comments.



Comment at: clang/include/clang/Sema/SemaConcept.h:48-52
+  if (ArgA.getKind() == TemplateArgument::Expression &&
+  ArgB.getKind() == TemplateArgument::Expression &&
+  ArgA.getAsExpr()->getType()->isUndeducedAutoType() &&
+  ArgB.getAsExpr()->getType()->isUndeducedAutoType())
+continue;

Why are looking at only the type of the expression?
The expression can be arbitrarily different, but as long as they are both 
undeduced auto, that is okay?



Comment at: clang/include/clang/Sema/SemaConcept.h:54-68
+  if (ArgA.getKind() == TemplateArgument::Type &&
+  ArgB.getKind() == TemplateArgument::Type)
+if (const auto *SubstA =
+ArgA.getAsType()->getAs())
+  if (const auto *SubstB =
+  ArgB.getAsType()->getAs()) {
+QualType ReplacementA = SubstA->getReplacementType();

It's a bit odd to find `SubstTemplateTypeParmType` necessary to implement the 
semantics of this change.

This is just type sugar we leave behind in the template instantiator to mark 
where type replacement happened.

There are several potential issues here:
1) This could be marking a substitution that happened at any template depth. Ie 
this could be marking a substitution from an outer template. Does the level not 
matter here at all? 
2) If the level does matter, you won't be able to reconstitute that easily 
without further improvements. See 
https://github.com/llvm/llvm-project/issues/55886 for example.
3) A substitution can replace a dependent type for another one, and when that 
other gets replaced, we lose track of that because `SubstTemplateTypeParmType` 
only holds a canonical underlying type.



Leaving that aside, I get the impression you are trying to work around the fact 
that when an expression appears in a canonical type, presumably because that 
expression is dependent on an NTTP, we can't rely on uniquing anymore to 
compare if they are the same type, as we lack in Clang the equivalent concept 
of canonicalization for expressions.

But this however is a bit hard to implement. Are we sure the standard requires 
this, or can we simply consider these types always different?



Comment at: clang/lib/AST/ASTContext.cpp:5149-5151
+Expr *E = new (*this)
+DeclRefExpr(*this, NTTP, /*enclosing*/ false, T,
+Expr::getValueKindForType(NTTPType), NTTP->getLocation());





Comment at: clang/lib/AST/ASTContext.cpp:5154-5156
+  E = new (*this) PackExpansionExpr(
+  NTTPType->isUndeducedAutoType() ? NTTPType : DependentTy, E,
+  NTTP->getLocation(), None);

I don't know if this change is necessary for this patch, as this looks part of 
the workaround in `SemaConcept.h`,
but I think a better way to preserve the type here might be to always use 
`NTTPType`, but then add an additional `Dependent` parameter to 
`PackExpansionExpr` which can be used to force the expression to be dependent.



Comment at: clang/lib/Sema/SemaConcept.cpp:827
+assert(FoldE->isRightFold() && FoldE->getOperator() == BO_LAnd);
+E = FoldE->getPattern();
+  }

If you need to dig down into the pattern, then I think the pattern might also 
contain casts and parenthesis which you would need to keep ignoring for the 
rest of the code to work.

I would consider adding a test for that.



Comment at: clang/lib/Sema/SemaOverload.cpp:9675-9676
-  // when comparing template functions. 
-  if (Cand1.Function && Cand2.Function && Cand1.Function->hasPrototype() &&
-  Cand2.Function->hasPrototype()) {
 auto *PT1 = cast(Cand1.Function->getFunctionType());

Why are these `hasPrototype` checks not needed anymore?

I don't see other changes which would obliviate the need for it. Presumably the 
code below is assuming we have them when we perform that `FunctionProtoType` 
cast.

Maybe this was either not possible, or we simply never added tests for it.



Comment at: clang/lib/Sema/SemaTemplate.cpp:1267
   BuildDeclRefExpr(NTTP, NTTP->getType(), VK_PRValue, NTTP->getLocation());
-  if (!Ref)
-return true;
+  assert(Ref);
   ExprResult ImmediatelyDeclaredConstraint = formImmediatelyDeclaredConstraint(

I don't think the `assert` is even necessary TBH, the function can't return 
nullptr.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5247-5248
+  for (unsigned i = 0; i < NumParams1; ++i)
+if (FD1->getParamDecl(i)->getType().getCanonicalType() !=
+FD2->getParamDecl(i)->getType().getCanonicalType())
+  return nullptr;





Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5256-5257
+  //   than the other.
+  if (TPOC == TPOC_Conversion && FD1->getReturnType().getCanonicalType() !=
+   

[clang] baa9eae - [NFC] fix incorrect indentation in docs

2022-09-04 Thread Chuanqi Xu via cfe-commits

Author: Chuanqi Xu
Date: 2022-09-05T11:05:23+08:00
New Revision: baa9eae279c1639f406015734ebbf4c429b15c21

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

LOG: [NFC] fix incorrect indentation in docs

Added: 


Modified: 
clang/docs/StandardCPlusPlusModules.rst

Removed: 




diff  --git a/clang/docs/StandardCPlusPlusModules.rst 
b/clang/docs/StandardCPlusPlusModules.rst
index ae434b14ef50c..86ba6f44dbb02 100644
--- a/clang/docs/StandardCPlusPlusModules.rst
+++ b/clang/docs/StandardCPlusPlusModules.rst
@@ -481,19 +481,19 @@ Then it is problematic if we remove ``foo.h`` before 
import `foo` module.
 
 .. code-block:: console
 
-  clang++ -std=c++20 foo.cppm --precompile  -o foo.pcm
-   mv foo.h foo.orig.h
+  $ clang++ -std=c++20 foo.cppm --precompile  -o foo.pcm
+  $ mv foo.h foo.orig.h
   # The following one is rejected
-   clang++ -std=c++20 Use.cpp -fmodule-file=foo.pcm -c
+  $ clang++ -std=c++20 Use.cpp -fmodule-file=foo.pcm -c
 
 The above case will rejected. And we're still able to workaround it by 
``-Xclang -fmodules-embed-all-files`` option:
 
 .. code-block:: console
 
-  clang++ -std=c++20 foo.cppm --precompile  -Xclang -fmodules-embed-all-files 
-o foo.pcm
-   mv foo.h foo.orig.h
-   clang++ -std=c++20 Use.cpp -fmodule-file=foo.pcm -c -o Use.o
-   clang++ Use.o foo.pcm
+  $ clang++ -std=c++20 foo.cppm --precompile  -Xclang 
-fmodules-embed-all-files -o foo.pcm
+  $ mv foo.h foo.orig.h
+  $ clang++ -std=c++20 Use.cpp -fmodule-file=foo.pcm -c -o Use.o
+  $ clang++ Use.o foo.pcm
 
 ABI Impacts
 ---



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


[PATCH] D132192: [RISCV] Add '32bit' feature to rv32 only builtins.

2022-09-04 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:4359
-return Diag(TheCall->getCallee()->getBeginLoc(),
-diag::err_32_bit_builtin_64_bit_tgt);
-

luismarques wrote:
> That tablegen def is still being used for X86. Maybe you could make a similar 
> patch for X86?
There's no "32bit" feature in X86.td in the backend. This was possible for 
RISC-V because I added "32bit" when I refactored mtune recently.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132192

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


[PATCH] D132952: [Sema] disable -Wvla for function array parameters

2022-09-04 Thread YingChi Long via Phabricator via cfe-commits
inclyc added inline comments.



Comment at: clang/test/Sema/warn-vla.c:8-12
+void test2(int n, int v[n]) { // c99 no-warning
+#if __STDC_VERSION__ < 199901L
+// expected-warning@-2{{variable length arrays are a C99 feature}}
+#endif
 }

aaron.ballman wrote:
> The diagnostic there is rather unfortunate because we're not using a 
> variable-length array in this case.
Emm, I'm not clear about whether we should consider this a VLA, and generates 
`-Wvla-extensions`. Is `v[n]` literally a variable-length array? (in source 
code) So it seems to me that we should still report c89 incompatibility 
warnings?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132952

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


[clang] 36a1ca5 - [ASTReader] Fix -Wunused-private-field in non-assertion builds after D128490. NFC

2022-09-04 Thread Fangrui Song via cfe-commits

Author: Fangrui Song
Date: 2022-09-04T23:48:55-07:00
New Revision: 36a1ca5835e0f7e0e02899d97cd2e4c7bf704361

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

LOG: [ASTReader] Fix -Wunused-private-field in non-assertion builds after 
D128490. NFC

Added: 


Modified: 
clang/lib/Serialization/ASTReader.cpp

Removed: 




diff  --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index 5d1ee45c10de5..568d948b541e5 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -9818,6 +9818,7 @@ void ASTReader::diagnoseOdrViolations() {
 }
 
 assert(Context.hasSameType(FirstField->getType(), SecondField->getType()));
+(void)Context;
 
 QualType FirstType = FirstField->getType();
 QualType SecondType = SecondField->getType();



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


[PATCH] D132607: [OffloadPackager] Add ability to extract images from other file types

2022-09-04 Thread Saiyedul Islam via Phabricator via cfe-commits
saiislam accepted this revision.
saiislam added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132607

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