[PATCH] D140339: [clang-format] Remove special logic for parsing concept definitions.

[Owen Pan via Phabricator via cfe-commits]

owenpan accepted this revision.
owenpan added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:23593-23594
-  "template \n"
-  "concept DelayedCheck = static_cast(0) ||\n"
-  "   requires(T t) { t.bar(); } && sizeof(T) <= 8;");
 

What would happen with parentheses added?
```
concept DelayedCheck = (static_cast(0) || requires(T t) { t.bar(); }) && 
sizeof(T) <= 8;
concept DelayedCheck = static_cast(0) || (requires(T t) { t.bar(); } && 
sizeof(T) <= 8);
```



Comment at: clang/unittests/Format/FormatTest.cpp:23946-23947
-  verifyFormat("template \n"
-   "concept C =\n"
-   "class X;");
 

Just want to make sure that the line break didn't matter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140339

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

[PATCH] D140775: [clangd] Hover: show CalleeArgInfo for literals

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

Do you want to add a simple test case for a non-literal expression? Something 
like hovering over the `+` in `2 + 3` should work.

Also, this is pre-existing, but I noticed that in a case like this:

  void bar(int);
  void func() {
int x = 42;
bar(x);
  }

the hover for `x` includes a line that just says `Passed` -- is that useful in 
any way? Should we just omit the CalleeArgInfo a case like that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140775

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

[PATCH] D140814: [clangd] show underlying type in type hint for `decltype(expr)`

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

Thanks, I think this behaviour change is reasonable.




Comment at: clang-tools-extra/clangd/InlayHints.cpp:667
+// print the underlying type for `decltype(expr)`
+if (T->isDecltypeType())
+  TypeHintPolicy.PrintCanonicalTypes = true;

Can we factor out a function `ShouldPrintCanonicalType(QualType)`, and add a 
comment like this in its implementation:

```
// The sugared type is more useful in some cases, and the canonical
// type in other cases. For now, prefer the sugared type unless
// we are printing `decltype(expr)`. This could be refined further
// (see https://github.com/clangd/clangd/issues/1298).
```

Then in this function we can set:

```
TypeHintPolicy.PrintCanonicalTypes = ShouldPrintCanonicalType(T);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140814

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

[PATCH] D140000: [clangd] Compute the unused includes in the check mode.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

what's the motivating reason behind the change?
we'll soon change the implementation to use include-cleaner instead, and can 
run the tool directly to verify the findings (rather than clangd). hence i feel 
like this will just turn into not-so-useful extra output for `clangd --check`, 
but maybe there's some other use case that i am missing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D14

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

[PATCH] D140793: [clang-tidy] Implement CppCoreGuideline CP.53

[Carlos Galvez via Phabricator via cfe-commits]

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

LGTM, thanks for the patch! Maybe wait a couple days before landing to give 
some extra time for other reviewers. You might also want to mark the comments 
as "Done", so reviewers can get a quick overview of unresolved issues (if any).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140793

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

[PATCH] D139458: [clangd] Full support for #import insertions

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/ASTSignals.h:39
+
+  static Symbol::IncludeDirective preferredIncludeDirective(
+  llvm::StringRef Filename, const LangOptions &LangOpts,

could you rather move this to `AST.h`, with some comments explaining what it 
does



Comment at: clang-tools-extra/clangd/ASTSignals.h:41
+  llvm::StringRef Filename, const LangOptions &LangOpts,
+  const IncludeStructure &Includes, ArrayRef TopLevelDecls);
 };

what about accepting an `llvm::ArrayRef MainFileIncludes` instead of 
the full `IncludeStructure`, as the former can be constructed a lot more easily.



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:395
+Symbol::IncludeDirective Directive = insertionDirective(Opts);
+tooling::IncludeDirective InsertDirective =
+Directive == Symbol::Import ? tooling::IncludeDirective::Import

nit: I'd actually inline this into use-side to not confuse the reader 
unnecessarily about whether this is marshalling/conversion



Comment at: clang-tools-extra/clangd/IncludeFixer.cpp:322
   llvm::StringSet<> InsertedHeaders;
+  auto InsertDirective = Directive == Symbol::IncludeDirective::Import ?
+  tooling::IncludeDirective::Import : tooling::IncludeDirective::Include;

again, i'd inline the marshalling



Comment at: clang-tools-extra/clangd/ParsedAST.cpp:566
   if (Preamble) {
+Includes = Preamble->Includes;
 for (const auto &Inc : Preamble->Includes.MainFileIncludes)

this is a somewhat heavy struct to copy (contains absolute paths to all the 
includes in the preamble). can you use a pointer here?



Comment at: clang-tools-extra/clangd/ParsedAST.h:88
   /// (These should be const, but RecursiveASTVisitor requires Decl*).
-  ArrayRef getLocalTopLevelDecls();
+  ArrayRef getLocalTopLevelDecls() const;
 

can you rather introduce a new overload that returns `ArrayRef`



Comment at: clang-tools-extra/clangd/unittests/ASTSignalsTests.cpp:87
+  HeaderTU.ExtraArgs = {"-xobjective-c++-header"};
+  Signals = ASTSignals::derive(HeaderTU.build());
+  EXPECT_EQ(Signals.InsertionDirective, Symbol::IncludeDirective::Import);

since we have a dedicated interface now, could you please test it instead (same 
for uses below)?



Comment at: clang-tools-extra/clangd/unittests/ASTSignalsTests.cpp:98
+  HeaderTU.Code = R"cpp(
+  #include "TestTU.h"
+

no need for the `#include` here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139458

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

[PATCH] D118743: [clang-tidy] Add `modernize-use-inline-const-variables-in-headers` check

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/modernize/UseInlineConstVariablesInHeadersCheck.cpp:21-23
+AST_MATCHER(NamedDecl, isInAnonymousNamespace) {
+  return Node.isInAnonymousNamespace();
+}

I recently added this matcher into ASTMatchers, so you can remove this custom 
matcher :) 



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize/use-inline-const-variables-in-headers.hpp:1
+// RUN: %check_clang_tidy %s -std=c++17 
modernize-use-inline-const-variables-in-headers %t
+

Should tests be added for the `CheckNonInline` and `CheckExtern` options?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118743

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

[PATCH] D137517: [TargetParser] Generate the defs for RISCV CPUs using llvm-tblgen.

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli updated this revision to Diff 485852.
fpetrogalli added a comment.

NFC - just an update on top of current `main`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  llvm/include/llvm/CMakeLists.txt
  llvm/include/llvm/TargetParser/CMakeLists.txt
  llvm/include/llvm/TargetParser/RISCVTargetParser.def
  llvm/include/llvm/TargetParser/RISCVTargetParser.h
  llvm/include/llvm/TargetParser/TargetParser.h
  llvm/include/llvm/module.extern.modulemap
  llvm/include/llvm/module.install.modulemap
  llvm/include/llvm/module.modulemap
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/lib/TargetParser/CMakeLists.txt
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/lib/TargetParser/TargetParser.cpp
  llvm/utils/TableGen/CMakeLists.txt
  llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
  llvm/utils/TableGen/TableGen.cpp
  llvm/utils/TableGen/TableGenBackends.h

Index: llvm/utils/TableGen/TableGenBackends.h
===
--- llvm/utils/TableGen/TableGenBackends.h
+++ llvm/utils/TableGen/TableGenBackends.h
@@ -94,7 +94,7 @@
 void EmitDirectivesDecl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDirectivesImpl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDXILOperation(RecordKeeper &RK, raw_ostream &OS);
-
+void EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS);
 } // End llvm namespace
 
 #endif
Index: llvm/utils/TableGen/TableGen.cpp
===
--- llvm/utils/TableGen/TableGen.cpp
+++ llvm/utils/TableGen/TableGen.cpp
@@ -58,6 +58,7 @@
   GenDirectivesEnumDecl,
   GenDirectivesEnumImpl,
   GenDXILOperation,
+  GenRISCVTargetDef,
 };
 
 namespace llvm {
@@ -141,8 +142,9 @@
 clEnumValN(GenDirectivesEnumImpl, "gen-directive-impl",
"Generate directive related implementation code"),
 clEnumValN(GenDXILOperation, "gen-dxil-operation",
-   "Generate DXIL operation information")));
-
+   "Generate DXIL operation information"),
+clEnumValN(GenRISCVTargetDef, "gen-riscv-target-def",
+   "Generate the list of CPU for RISCV")));
 cl::OptionCategory PrintEnumsCat("Options for -print-enums");
 cl::opt Class("class", cl::desc("Print Enum list for this class"),
cl::value_desc("class name"),
@@ -278,6 +280,9 @@
   case GenDXILOperation:
 EmitDXILOperation(Records, OS);
 break;
+  case GenRISCVTargetDef:
+EmitRISCVTargetDef(Records, OS);
+break;
   }
 
   return false;
Index: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
===
--- /dev/null
+++ llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
@@ -0,0 +1,62 @@
+//===- RISCVTargetDefEmitter.cpp - Generate lists of RISCV CPUs ---===//
+//
+// 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 tablegen backend emits the include file needed by the target
+// parser to parse the RISC-V CPUs.
+//
+//===--===//
+
+#include "TableGenBackends.h"
+#include "llvm/TableGen/Record.h"
+
+using namespace llvm;
+
+static std::string getEnumFeatures(const Record &Rec) {
+  std::vector Features = Rec.getValueAsListOfDefs("Features");
+  if (find_if(Features, [](const Record *R) {
+return R->getName() == "Feature64Bit";
+  }) != Features.end())
+return "FK_64BIT";
+
+  return "FK_NONE";
+}
+
+void llvm::EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS) {
+  using MapTy = std::pair>;
+  using RecordMap = std::map, std::less<>>;
+  const RecordMap &Map = RK.getDefs();
+
+  OS << "#ifndef PROC\n"
+ << "#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)\n"
+ << "#endif\n\n";
+
+  OS << "PROC(INVALID, {\"invalid\"}, FK_INVALID, {\"\"})\n";
+  // Iterate on all definition records.
+  for (const MapTy &Def : Map) {
+const Record &Rec = *(Def.second);
+if (Rec.isSubClassOf("RISCVProcessorModelPROC"))
+  OS << "PROC(" << Rec.getName() << ", "
+ << "{\"" << Rec.getValueAsString("Name") << "\"},"
+ << getEnumFeatures(Rec) << ", "
+ << "{\"" << Rec.getValueAsString("DefaultMarch") << "\"})\n";
+  }
+  OS << "\n#undef PROC\n";
+  OS << "\n";
+  OS << "#ifndef TUNE_PROC\n"
+ << "#define TUNE_PROC(ENUM, NAME)\n"
+ << "#endif\n\n";
+  OS << "TUNE_PROC(GENERIC, \"generic\")\n";
+  for (const MapTy &Def : Map) {
+const Record &Rec = *(Def.second);
+if (Rec.isSubClassOf("RISCVProcessorModelTUNE_

[PATCH] D140018: [clang-tidy] Support std::string_view in readability-redundant-string-cstr

[Tamas Berghammer via Phabricator via cfe-commits]

tberghammer updated this revision to Diff 485853.
tberghammer added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140018

Files:
  clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
@@ -48,6 +48,15 @@
 typedef basic_string, std::allocator> wstring;
 typedef basic_string, std::allocator> u16string;
 typedef basic_string, std::allocator> u32string;
+
+template 
+struct basic_string_view {
+  basic_string_view(const C* s);
+};
+typedef basic_string_view> string_view;
+typedef basic_string_view> wstring_view;
+typedef basic_string_view> u16string_view;
+typedef basic_string_view> u32string_view;
 }
 
 std::string operator+(const std::string&, const std::string&);
@@ -169,6 +178,15 @@
   tmp.insert(1, s);
   tmp.insert(1, s.c_str(), 2);
 }
+void f7(std::string_view sv) {
+  std::string s;
+  f7(s.c_str());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}f7(s);{{$}}
+  f7(s.data());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'data' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}f7(s);{{$}}
+}
 
 // Tests for std::wstring.
 
@@ -177,6 +195,15 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
   // CHECK-FIXES: {{^  }}g1(s);{{$}}
 }
+void g2(std::wstring_view sv) {
+  std::wstring s;
+  g2(s.c_str());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}g2(s);{{$}}
+  g2(s.data());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'data' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}g2(s);{{$}}
+}
 
 // Tests for std::u16string.
 
@@ -185,6 +212,15 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
   // CHECK-FIXES: {{^  }}h1(s);{{$}}
 }
+void h2(std::u16string_view sv) {
+  std::u16string s;
+  h2(s.c_str());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}h2(s);{{$}}
+  h2(s.data());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'data' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}h2(s);{{$}}
+}
 
 // Tests for std::u32string.
 
@@ -193,6 +229,15 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
   // CHECK-FIXES: {{^  }}k1(s);{{$}}
 }
+void k2(std::u32string_view sv) {
+  std::u32string s;
+  k2(s.c_str());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}k2(s);{{$}}
+  k2(s.data());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'data' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}k2(s);{{$}}
+}
 
 // Tests on similar classes that aren't good candidates for this checker.
 
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -204,6 +204,10 @@
   The check now skips concept definitions since redundant expressions still make sense
   inside them.
 
+- Support removing ``c_str`` calls from ``std::string_view`` constructor calls in
+  :doc: `readability-redundant-string-cstr `
+  check.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
@@ -86,6 +86,11 @@
   // be present explicitly.
   hasArgument(1, cxxDefaultArgExpr();
 
+  // Match string constructor.
+  const auto StringViewConstructorExpr = cxxConstructExpr(
+  argumentCountIs(1),
+  hasDeclaration(cxxMethodDecl(hasName("basic_string_view";
+
   // Match a call to the string 'c_str()' method.
   const auto StringCStrCallExpr =
   cxxMemberCallExpr(on(StringExpr.bind("arg")),
@@ -101,7 +106,8 @@
   traverse(
   TK_AsIs,
   cxxConstructExpr(
-  StringConstructorExpr, hasArgument(0, StringCStrCallExpr),
+  anyOf(StringConstructorExpr, Strin

[PATCH] D140814: [clangd] show underlying type in type hint for `decltype(expr)`

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 485854.
v1nh1shungry added a comment.

apply comment's suggestions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140814

Files:
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1364,7 +1364,6 @@
 TEST(TypeHints, Decltype) {
   assertTypeHints(R"cpp(
 $a[[decltype(0)]] a;
-// FIXME: will be nice to show `: int` instead
 $b[[decltype(a)]] b;
 const $c[[decltype(0)]] &c = b;
 
@@ -1377,11 +1376,13 @@
 
 template  struct Foo;
 using G = Foo<$g[[decltype(0)]], float>;
+
+auto $h[[h]] = $i[[decltype(0)]]{};
   )cpp",
-  ExpectedHint{": int", "a"},
-  ExpectedHint{": decltype(0)", "b"},
+  ExpectedHint{": int", "a"}, ExpectedHint{": int", "b"},
   ExpectedHint{": int", "c"}, ExpectedHint{": int", "e"},
-  ExpectedHint{": int", "f"}, ExpectedHint{": int", "g"});
+  ExpectedHint{": int", "f"}, ExpectedHint{": int", "g"},
+  ExpectedHint{": int", "h"}, ExpectedHint{": int", "i"});
 }
 
 TEST(DesignatorHints, Basic) {
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -662,7 +662,21 @@
  sourceLocToPosition(SM, Spelled->back().endLocation())};
   }
 
+  // The sugared type is more useful in some cases, and the canonical
+  // type in other cases. For now, prefer the sugared type unless
+  // we are printing `decltype(expr)`. This could be refined further
+  // (see https://github.com/clangd/clangd/issues/1298).
+  static bool shouldPrintCanonicalType(QualType QT) {
+if (QT->isDecltypeType())
+  return true;
+if (const AutoType *AT = QT->getContainedAutoType())
+  if (AT->getDeducedType()->isDecltypeType())
+return true;
+return false;
+  }
+
   void addTypeHint(SourceRange R, QualType T, llvm::StringRef Prefix) {
+TypeHintPolicy.PrintCanonicalTypes = shouldPrintCanonicalType(T);
 addTypeHint(R, T, Prefix, TypeHintPolicy);
   }
 


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1364,7 +1364,6 @@
 TEST(TypeHints, Decltype) {
   assertTypeHints(R"cpp(
 $a[[decltype(0)]] a;
-// FIXME: will be nice to show `: int` instead
 $b[[decltype(a)]] b;
 const $c[[decltype(0)]] &c = b;
 
@@ -1377,11 +1376,13 @@
 
 template  struct Foo;
 using G = Foo<$g[[decltype(0)]], float>;
+
+auto $h[[h]] = $i[[decltype(0)]]{};
   )cpp",
-  ExpectedHint{": int", "a"},
-  ExpectedHint{": decltype(0)", "b"},
+  ExpectedHint{": int", "a"}, ExpectedHint{": int", "b"},
   ExpectedHint{": int", "c"}, ExpectedHint{": int", "e"},
-  ExpectedHint{": int", "f"}, ExpectedHint{": int", "g"});
+  ExpectedHint{": int", "f"}, ExpectedHint{": int", "g"},
+  ExpectedHint{": int", "h"}, ExpectedHint{": int", "i"});
 }
 
 TEST(DesignatorHints, Basic) {
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -662,7 +662,21 @@
  sourceLocToPosition(SM, Spelled->back().endLocation())};
   }
 
+  // The sugared type is more useful in some cases, and the canonical
+  // type in other cases. For now, prefer the sugared type unless
+  // we are printing `decltype(expr)`. This could be refined further
+  // (see https://github.com/clangd/clangd/issues/1298).
+  static bool shouldPrintCanonicalType(QualType QT) {
+if (QT->isDecltypeType())
+  return true;
+if (const AutoType *AT = QT->getContainedAutoType())
+  if (AT->getDeducedType()->isDecltypeType())
+return true;
+return false;
+  }
+
   void addTypeHint(SourceRange R, QualType T, llvm::StringRef Prefix) {
+TypeHintPolicy.PrintCanonicalTypes = shouldPrintCanonicalType(T);
 addTypeHint(R, T, Prefix, TypeHintPolicy);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140814: [clangd] show underlying type in type hint for `decltype(expr)`

[Vincent Hong via Phabricator via cfe-commits]

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

Thanks for reviewing and giving suggestions!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140814

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

[PATCH] D139534: [analyzer] Don't escape local static memregions on bind

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

steakhal added a comment.

Let me know if you have further concerns. @xazax.hun @NoQ


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139534

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

[clang-tools-extra] f325b5b - [clang-tidy] Support std::string_view in readability-redundant-string-cstr

[Tamas Berghammer via cfe-commits]

Author: Tamas Berghammer
Date: 2023-01-02T06:24:00-05:00
New Revision: f325b5b8cb1d6c16dd04c88380caea7b0a7750eb

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

LOG: [clang-tidy] Support std::string_view in readability-redundant-string-cstr

Previously we were matching constructor calls for std::string and
llvm::StringRef and this change extends this set with including
std::string_view as well.

Reviewed By: njames93, carlosgalvezp

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst

clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
index 79211731ff7be..9ecd6d612e0c8 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
@@ -86,6 +86,11 @@ void RedundantStringCStrCheck::registerMatchers(
   // be present explicitly.
   hasArgument(1, cxxDefaultArgExpr();
 
+  // Match string constructor.
+  const auto StringViewConstructorExpr = cxxConstructExpr(
+  argumentCountIs(1),
+  hasDeclaration(cxxMethodDecl(hasName("basic_string_view";
+
   // Match a call to the string 'c_str()' method.
   const auto StringCStrCallExpr =
   cxxMemberCallExpr(on(StringExpr.bind("arg")),
@@ -101,7 +106,8 @@ void RedundantStringCStrCheck::registerMatchers(
   traverse(
   TK_AsIs,
   cxxConstructExpr(
-  StringConstructorExpr, hasArgument(0, StringCStrCallExpr),
+  anyOf(StringConstructorExpr, StringViewConstructorExpr),
+  hasArgument(0, StringCStrCallExpr),
   unless(anyOf(HasRValueTempParent, hasParent(cxxBindTemporaryExpr(
 HasRValueTempParent)),
   this);

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 2135dd1e01be0..580776b76fd14 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -204,6 +204,10 @@ Changes in existing checks
   The check now skips concept definitions since redundant expressions still 
make sense
   inside them.
 
+- Support removing ``c_str`` calls from ``std::string_view`` constructor calls 
in
+  :doc: `readability-redundant-string-cstr 
`
+  check.
+
 Removed checks
 ^^
 

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
index e1df810b3..ed50ad16f2423 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
@@ -48,6 +48,15 @@ typedef basic_string, 
std::allocator> string;
 typedef basic_string, 
std::allocator> wstring;
 typedef basic_string, std::allocator> 
u16string;
 typedef basic_string, std::allocator> 
u32string;
+
+template 
+struct basic_string_view {
+  basic_string_view(const C* s);
+};
+typedef basic_string_view> string_view;
+typedef basic_string_view> wstring_view;
+typedef basic_string_view> u16string_view;
+typedef basic_string_view> u32string_view;
 }
 
 std::string operator+(const std::string&, const std::string&);
@@ -169,6 +178,15 @@ void f6(const std::string &s) {
   tmp.insert(1, s);
   tmp.insert(1, s.c_str(), 2);
 }
+void f7(std::string_view sv) {
+  std::string s;
+  f7(s.c_str());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str' 
[readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}f7(s);{{$}}
+  f7(s.data());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'data' 
[readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}f7(s);{{$}}
+}
 
 // Tests for std::wstring.
 
@@ -177,6 +195,15 @@ void g1(const std::wstring &s) {
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str' 
[readability-redundant-string-cstr]
   // CHECK-FIXES: {{^  }}g1(s);{{$}}
 }
+void g2(std::wstring_view sv) {
+  std::wstring s;
+  g2(s.c_str());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str' 
[readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}g2(s);{{$}}
+  g2(s.data());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'data' 
[readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}g2(s);{{$}}
+}
 
 // Tests for std::u16string.
 
@@ -185,6 +212,15 @@ void h1(const std::u

[PATCH] D140018: [clang-tidy] Support std::string_view in readability-redundant-string-cstr

[Tamas Berghammer 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 rGf325b5b8cb1d: [clang-tidy] Support std::string_view in 
readability-redundant-string-cstr (authored by tberghammer).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140018

Files:
  clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
@@ -48,6 +48,15 @@
 typedef basic_string, std::allocator> wstring;
 typedef basic_string, std::allocator> u16string;
 typedef basic_string, std::allocator> u32string;
+
+template 
+struct basic_string_view {
+  basic_string_view(const C* s);
+};
+typedef basic_string_view> string_view;
+typedef basic_string_view> wstring_view;
+typedef basic_string_view> u16string_view;
+typedef basic_string_view> u32string_view;
 }
 
 std::string operator+(const std::string&, const std::string&);
@@ -169,6 +178,15 @@
   tmp.insert(1, s);
   tmp.insert(1, s.c_str(), 2);
 }
+void f7(std::string_view sv) {
+  std::string s;
+  f7(s.c_str());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}f7(s);{{$}}
+  f7(s.data());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'data' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}f7(s);{{$}}
+}
 
 // Tests for std::wstring.
 
@@ -177,6 +195,15 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
   // CHECK-FIXES: {{^  }}g1(s);{{$}}
 }
+void g2(std::wstring_view sv) {
+  std::wstring s;
+  g2(s.c_str());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}g2(s);{{$}}
+  g2(s.data());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'data' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}g2(s);{{$}}
+}
 
 // Tests for std::u16string.
 
@@ -185,6 +212,15 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
   // CHECK-FIXES: {{^  }}h1(s);{{$}}
 }
+void h2(std::u16string_view sv) {
+  std::u16string s;
+  h2(s.c_str());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}h2(s);{{$}}
+  h2(s.data());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'data' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}h2(s);{{$}}
+}
 
 // Tests for std::u32string.
 
@@ -193,6 +229,15 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
   // CHECK-FIXES: {{^  }}k1(s);{{$}}
 }
+void k2(std::u32string_view sv) {
+  std::u32string s;
+  k2(s.c_str());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}k2(s);{{$}}
+  k2(s.data());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'data' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}k2(s);{{$}}
+}
 
 // Tests on similar classes that aren't good candidates for this checker.
 
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -204,6 +204,10 @@
   The check now skips concept definitions since redundant expressions still make sense
   inside them.
 
+- Support removing ``c_str`` calls from ``std::string_view`` constructor calls in
+  :doc: `readability-redundant-string-cstr `
+  check.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
@@ -86,6 +86,11 @@
   // be present explicitly.
   hasArgument(1, cxxDefaultArgExpr();
 
+  // Match string constructor.
+  const auto StringViewConstructorExpr = cxxConstructExpr(
+  argumentCountIs(1),
+  hasDeclaration(cxxMethodDecl(hasName("basic_string_view";
+
   // Match a call to the string 'c_str()' method.
   const auto StringCStrCallExpr =
   cxxMemberCallExpr(on(StringExpr.bind("arg")),
@@ -101,7 +106,8 @@
   trav

[PATCH] D140018: [clang-tidy] Support std::string_view in readability-redundant-string-cstr

[Tamas Berghammer via Phabricator via cfe-commits]

tberghammer added a comment.

Sorry, I was away during the holidays but just pushed the change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140018

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

[PATCH] D140532: .clang-tidy: disable misc-use-anonymous-namespace check.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D140532#4012879 , @sammccall wrote:

> Yeah, good call. Do you mind if I first use this as a trigger to start a 
> thread about this style rule on discourse, though? I've been meaning to, it 
> came up in a review recently.

sure, please go ahead. (I found that these warnings are annoying when editing 
the llvm code file).

> I think there's a fair chance of changing the policy from "require static" to 
> "allow either" or "require anon namespace", and having the issue "live" in 
> clang-tidy might make for a more engaged discussion.

Yeah, the code style says `require static`, I think in practice we allow either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140532

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

[PATCH] D136554: Implement CWG2631

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

In D136554#4020387 , @MaskRay wrote:

> @rupprecht You may consider contributing some interesting tests (which work 
> before this patch) in a separate patch:)

Well, all the failures reported have been reduced and added as test cases.
The coverage for member initializers was a bit spotty, that's for sure!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136554

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

[PATCH] D137517: [TargetParser] Generate the defs for RISCV CPUs using llvm-tblgen.

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli updated this revision to Diff 485858.
fpetrogalli added a comment.

Fix pre-merge check at 
https://buildkite.com/llvm-project/premerge-checks/builds/128468#0185724f-5ade-4603-99ce-f417efb546e8
 (missing header inclusion):

  
C:\ws\w3\llvm-project\premerge-checks\llvm\include\llvm/TargetParser/RISCVTargetParser.h(44):
 error C2039: 'vector': is not a member of 'std'
  C:\BuildTools\VC\Tools\MSVC\14.29.30133\include\string(24): note: see 
declaration of 'std'
  
C:\ws\w3\llvm-project\premerge-checks\llvm\include\llvm/TargetParser/RISCVTargetParser.h(44):
 error C2061: syntax error: identifier 'vector'


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  llvm/include/llvm/CMakeLists.txt
  llvm/include/llvm/TargetParser/CMakeLists.txt
  llvm/include/llvm/TargetParser/RISCVTargetParser.def
  llvm/include/llvm/TargetParser/RISCVTargetParser.h
  llvm/include/llvm/TargetParser/TargetParser.h
  llvm/include/llvm/module.extern.modulemap
  llvm/include/llvm/module.install.modulemap
  llvm/include/llvm/module.modulemap
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/lib/TargetParser/CMakeLists.txt
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/lib/TargetParser/TargetParser.cpp
  llvm/utils/TableGen/CMakeLists.txt
  llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
  llvm/utils/TableGen/TableGen.cpp
  llvm/utils/TableGen/TableGenBackends.h

Index: llvm/utils/TableGen/TableGenBackends.h
===
--- llvm/utils/TableGen/TableGenBackends.h
+++ llvm/utils/TableGen/TableGenBackends.h
@@ -94,7 +94,7 @@
 void EmitDirectivesDecl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDirectivesImpl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDXILOperation(RecordKeeper &RK, raw_ostream &OS);
-
+void EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS);
 } // End llvm namespace
 
 #endif
Index: llvm/utils/TableGen/TableGen.cpp
===
--- llvm/utils/TableGen/TableGen.cpp
+++ llvm/utils/TableGen/TableGen.cpp
@@ -58,6 +58,7 @@
   GenDirectivesEnumDecl,
   GenDirectivesEnumImpl,
   GenDXILOperation,
+  GenRISCVTargetDef,
 };
 
 namespace llvm {
@@ -141,8 +142,9 @@
 clEnumValN(GenDirectivesEnumImpl, "gen-directive-impl",
"Generate directive related implementation code"),
 clEnumValN(GenDXILOperation, "gen-dxil-operation",
-   "Generate DXIL operation information")));
-
+   "Generate DXIL operation information"),
+clEnumValN(GenRISCVTargetDef, "gen-riscv-target-def",
+   "Generate the list of CPU for RISCV")));
 cl::OptionCategory PrintEnumsCat("Options for -print-enums");
 cl::opt Class("class", cl::desc("Print Enum list for this class"),
cl::value_desc("class name"),
@@ -278,6 +280,9 @@
   case GenDXILOperation:
 EmitDXILOperation(Records, OS);
 break;
+  case GenRISCVTargetDef:
+EmitRISCVTargetDef(Records, OS);
+break;
   }
 
   return false;
Index: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
===
--- /dev/null
+++ llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
@@ -0,0 +1,62 @@
+//===- RISCVTargetDefEmitter.cpp - Generate lists of RISCV CPUs ---===//
+//
+// 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 tablegen backend emits the include file needed by the target
+// parser to parse the RISC-V CPUs.
+//
+//===--===//
+
+#include "TableGenBackends.h"
+#include "llvm/TableGen/Record.h"
+
+using namespace llvm;
+
+static std::string getEnumFeatures(const Record &Rec) {
+  std::vector Features = Rec.getValueAsListOfDefs("Features");
+  if (find_if(Features, [](const Record *R) {
+return R->getName() == "Feature64Bit";
+  }) != Features.end())
+return "FK_64BIT";
+
+  return "FK_NONE";
+}
+
+void llvm::EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS) {
+  using MapTy = std::pair>;
+  using RecordMap = std::map, std::less<>>;
+  const RecordMap &Map = RK.getDefs();
+
+  OS << "#ifndef PROC\n"
+ << "#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)\n"
+ << "#endif\n\n";
+
+  OS << "PROC(INVALID, {\"invalid\"}, FK_INVALID, {\"\"})\n";
+  // Iterate on all definition records.
+  for (const MapTy &Def : Map) {
+const Record &Rec = *(Def.second);
+if (Rec.isSubClassOf("RISCVProcessorModelPROC"))
+  OS << "PROC(" << Rec.getName() << ",

[PATCH] D140793: [clang-tidy] Implement CppCoreGuideline CP.53

[Chris Cotter via Phabricator via cfe-commits]

ccotter marked 4 inline comments as done.
ccotter added a comment.

In D140793#4021694 , @carlosgalvezp 
wrote:

> LGTM, thanks for the patch! Maybe wait a couple days before landing to give 
> some extra time for other reviewers. You might also want to mark the comments 
> as "Done", so reviewers can get a quick overview of unresolved issues (if 
> any).

Done, and thanks! I don't have permissions to land, so could you help land this 
for me after a few days have passed (assuming now further feedback)? You can 
use `Chris Cotter `


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140793

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

[PATCH] D140387: [clang][analyzer] Add stream related functions to StdLibraryFunctionsChecker.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Would be possible to test the errno specific changes as well?




Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp:310
+  const MemRegion *ErrnoR = State->get();
+  if (!ErrnoR)
+return State;

When can this occur? Maybe we can turn this early return to an assert -- when 
`ModelPOSIX` is true, this mustn't be null (if what I'm saying is correct).



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:839
+  Result += DK == Violation ? " should not be zero" : " is not zero";
+  return Result.c_str();
+}

Just thinking aloud, no changes required here necesserily -- in most places, we 
use a combination of `SmallString` and `raw_svector_ostream` so construct 
strings, but I can't find anything set in stone about the practice (mostly 
looking at [[ 
https://llvm.org/docs/ProgrammersManual.html#string-like-containers | LLVM's 
Programmers Manual ]]). Isn't this just good enough? Is `llvm::Twine` worth the 
hassle?



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1691-1702
+  .Case({ArgumentCondition(1U, WithinRange, Range(1, SizeMax)),
+ ArgumentCondition(2U, WithinRange, Range(1, SizeMax)),
+ ReturnValueCondition(BO_LT, ArgNo(2)),
  ReturnValueCondition(WithinRange, Range(0, SizeMax))},
-ErrnoIrrelevant)
+ErrnoNEZeroIrrelevant)
+  .Case({ArgumentCondition(1U, WithinRange, Range(1, SizeMax)),
+ ReturnValueCondition(BO_EQ, ArgNo(2)),

If I undetstand correctly, these 3 cases will cause the current path of 
execution to split into 3. I vaguely recall some arguments against this, but 
that's been a while, did the stance on this change? I see a number of already 
commited summaries with 3 cases, so this could be fine for all I know.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1705-1707
+  // FIXME: It should be allowed to have a null buffer if any of
+  // args 1 or 2 are zero. Remove NotNull check of arg 0, add a check
+  // for non-null buffer if non-zero size to BufferSizeConstraint?

Maybe `StreamChecker` could take over the handling of that case? Seems like it 
also warns about the nullness of the first `fread` parameter.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1848-1849
+// int fseek(FILE *stream, long offset, int whence);
+// FIXME: It is possible to get the 'SEEK_' values (like EOFv) for arg 2
+// condition.
+addToFunctionSummaryMap(

Sure, but what should be fixed? We should check whether the the last argument 
is a `SEEK_*` value?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140387

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

[PATCH] D140757: [Hexagon][VE][WebAssembly] Define __GCC_HAVE_SYNC_COMPARE_AND_SWAP macros

[Krzysztof Parzyszek via Phabricator via cfe-commits]

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

All of Hexagon's atomics are implemented using load-locked/store-conditional.  
If that meets the expectations of these macros, then this is fine with me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140757

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

[PATCH] D112680: [OpenMP] Lower printf to __llvm_omp_vprintf

[Ron Lieberman via Phabricator via cfe-commits]

ronlieb added a comment.
Herald added subscribers: kosarev, mattd.
Herald added a project: All.

@JonChesterfield   i am seeing the testcase bug50022.cpp occasionally randomly 
failing in amdgpu bbot
most recently:   https://lab.llvm.org/buildbot/#/builders/193/builds/24230

the same bbot in Staging passed on the patch

the test hangs, and after 23 minutes or so, the bbot terminates.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112680

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

[PATCH] D140843: [clang-format] fix template closer followed by >

[Zhikai Zeng via Phabricator via cfe-commits]

Backl1ght created this revision.
Backl1ght added reviewers: HazardyKnusperkeks, MyDeveloperDay.
Backl1ght added a project: clang-format.
Herald added a project: All.
Backl1ght requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch is to fix https://github.com/llvm/llvm-project/issues/59785.

related patches: https://reviews.llvm.org/D86581 and 
https://reviews.llvm.org/D100778.

I think the condition added in D100778  is 
enough to judge shifts within conditions, the condition added in D86581 
 is unnecessary and will cause template 
opener/closer parsed as binary operator, so I simply remove the condition added 
in D86581 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140843

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10341,6 +10341,8 @@
   verifyFormat("bool_constant");
   verifyFormat("bool_constant");
 
+  verifyFormat("if (std::tuple_size_v > 0)");
+
   // Not template parameters.
   verifyFormat("return a < b && c > d;");
   verifyFormat("void f() {\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -166,10 +166,9 @@
 // parameter cases, but should not alter program semantics.
 if (CurrentToken->Next && CurrentToken->Next->is(tok::greater) &&
 Left->ParentBracket != tok::less &&
-(isKeywordWithCondition(*Line.First) ||
- CurrentToken->getStartOfNonWhitespace() ==
- 
CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset(
- -1))) {
+CurrentToken->getStartOfNonWhitespace() ==
+CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset(
+-1)) {
   return false;
 }
 Left->MatchingParen = CurrentToken;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10341,6 +10341,8 @@
   verifyFormat("bool_constant");
   verifyFormat("bool_constant");
 
+  verifyFormat("if (std::tuple_size_v > 0)");
+
   // Not template parameters.
   verifyFormat("return a < b && c > d;");
   verifyFormat("void f() {\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -166,10 +166,9 @@
 // parameter cases, but should not alter program semantics.
 if (CurrentToken->Next && CurrentToken->Next->is(tok::greater) &&
 Left->ParentBracket != tok::less &&
-(isKeywordWithCondition(*Line.First) ||
- CurrentToken->getStartOfNonWhitespace() ==
- CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset(
- -1))) {
+CurrentToken->getStartOfNonWhitespace() ==
+CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset(
+-1)) {
   return false;
 }
 Left->MatchingParen = CurrentToken;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140387: [clang][analyzer] Add stream related functions to StdLibraryFunctionsChecker.

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

balazske added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp:310
+  const MemRegion *ErrnoR = State->get();
+  if (!ErrnoR)
+return State;

Szelethus wrote:
> When can this occur? Maybe we can turn this early return to an assert -- when 
> `ModelPOSIX` is true, this mustn't be null (if what I'm saying is correct).
I am not totally sure that `errno` is found always (definition of `errno` is 
encountered) if a summary for a standard function is added. The standard 
function in itself must be there, otherwise no summary is added and this 
function should not be called. But definition of `errno` may reside in other 
header (errno.h) that is probably not included in the translation unit. If the 
errno definition is not found the `ErrnoRegion` is not set (the checker may 
still turned be on but does nothing, but these functions are allowed to be 
called). Existence of `errno` is not even dependent on `ModelPOSIX`, `errno` is 
defined in the (non POSIX) C standard.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:839
+  Result += DK == Violation ? " should not be zero" : " is not zero";
+  return Result.c_str();
+}

Szelethus wrote:
> Just thinking aloud, no changes required here necesserily -- in most places, 
> we use a combination of `SmallString` and `raw_svector_ostream` so construct 
> strings, but I can't find anything set in stone about the practice (mostly 
> looking at [[ 
> https://llvm.org/docs/ProgrammersManual.html#string-like-containers | LLVM's 
> Programmers Manual ]]). Isn't this just good enough? Is `llvm::Twine` worth 
> the hassle?
The `Twine` may work is all partial strings are constant pointers, this is not 
true here. The `getArgDesc` does not return a constant string, the returned 
value must be copied. Still it may work if one single expression is used to 
construct the string.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1691-1702
+  .Case({ArgumentCondition(1U, WithinRange, Range(1, SizeMax)),
+ ArgumentCondition(2U, WithinRange, Range(1, SizeMax)),
+ ReturnValueCondition(BO_LT, ArgNo(2)),
  ReturnValueCondition(WithinRange, Range(0, SizeMax))},
-ErrnoIrrelevant)
+ErrnoNEZeroIrrelevant)
+  .Case({ArgumentCondition(1U, WithinRange, Range(1, SizeMax)),
+ ReturnValueCondition(BO_EQ, ArgNo(2)),

Szelethus wrote:
> If I undetstand correctly, these 3 cases will cause the current path of 
> execution to split into 3. I vaguely recall some arguments against this, but 
> that's been a while, did the stance on this change? I see a number of already 
> commited summaries with 3 cases, so this could be fine for all I know.
The problematic case is if the function is not evaluated by `StreamChecker`, 
otherwise `StreamChecker` already sets the constraints on the return value and 
argument and some of the cases here should become unsatisfied. It may be known 
about the argument 1 that it can not be zero (or is only zero), then it is only 
2 possible branches. (Still it can happen often that all 3 branches are 
possible.) This split is needed to make the summary correct and not dependent 
on if `StreamChecker` evaluates the function or not.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1705-1707
+  // FIXME: It should be allowed to have a null buffer if any of
+  // args 1 or 2 are zero. Remove NotNull check of arg 0, add a check
+  // for non-null buffer if non-zero size to BufferSizeConstraint?

Szelethus wrote:
> Maybe `StreamChecker` could take over the handling of that case? Seems like 
> it also warns about the nullness of the first `fread` parameter.
The current approach was that the NULL argument warning is generated by one 
checker only, `StreamChecker` or this checker. The NULL check is already built 
into these summaries so this is the better place for the warning. The NULL 
warning is removed from `StreamChecker` in D137790. Additionally the same 
problem can happen with `BufferSizeConstraint` at other function summaries, we 
can say generally (or more often) that if a buffer has size of 0 it is allowed 
to be NULL pointer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140387

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

[PATCH] D140845: [clang][Interp] Fix left-/right-shifting more than sizeof(unsigned)

[Timm Bäder via Phabricator via cfe-commits]

tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, tahonermann, shafik.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

  We were just casting to `unsigned` before, so that caused problems when
  shifting more bits than `unsigned` has.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140845

Files:
  clang/lib/AST/Interp/Integral.h
  clang/lib/AST/Interp/Interp.h
  clang/test/AST/Interp/shifts.cpp


Index: clang/test/AST/Interp/shifts.cpp
===
--- clang/test/AST/Interp/shifts.cpp
+++ clang/test/AST/Interp/shifts.cpp
@@ -143,4 +143,8 @@
 
   constexpr char c2 = 1;
   constexpr int i3 = c2 << (__CHAR_BIT__ + 1); // Not ill-formed
+
+  constexpr signed long int L = 1;
+  constexpr signed int R = 62;
+  constexpr decltype(L) M = L << R;
 };
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -1321,8 +1321,9 @@
   if (!CheckShift(S, OpPC, RHS, Bits))
 return false;
 
-  unsigned URHS = static_cast(RHS);
-  S.Stk.push(LT::from(static_cast(LHS) >> URHS, LHS.bitWidth()));
+  Integral R;
+  Integral::shiftRight(LHS.toUnsigned(), RHS, Bits, &R);
+  S.Stk.push(R);
   return true;
 }
 
@@ -1337,9 +1338,9 @@
   if (!CheckShift(S, OpPC, RHS, Bits))
 return false;
 
-  unsigned URHS = static_cast(RHS);
-  S.Stk.push(LT::from(static_cast(LHS) << URHS, LHS.bitWidth()));
-
+  Integral R;
+  Integral::shiftLeft(LHS.toUnsigned(), RHS, Bits, &R);
+  S.Stk.push(R);
   return true;
 }
 
Index: clang/lib/AST/Interp/Integral.h
===
--- clang/lib/AST/Interp/Integral.h
+++ clang/lib/AST/Interp/Integral.h
@@ -241,6 +241,18 @@
 return false;
   }
 
+  template 
+  static void shiftLeft(const Integral A, const Integral B,
+unsigned OpBits, Integral *R) {
+*R = Integral::from(A.V << B.V, OpBits);
+  }
+
+  template 
+  static void shiftRight(const Integral A, const Integral B,
+ unsigned OpBits, Integral *R) {
+*R = Integral::from(A.V >> B.V, OpBits);
+  }
+
 private:
   template  static bool CheckAddUB(T A, T B, T &R) {
 if constexpr (std::is_signed_v) {


Index: clang/test/AST/Interp/shifts.cpp
===
--- clang/test/AST/Interp/shifts.cpp
+++ clang/test/AST/Interp/shifts.cpp
@@ -143,4 +143,8 @@
 
   constexpr char c2 = 1;
   constexpr int i3 = c2 << (__CHAR_BIT__ + 1); // Not ill-formed
+
+  constexpr signed long int L = 1;
+  constexpr signed int R = 62;
+  constexpr decltype(L) M = L << R;
 };
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -1321,8 +1321,9 @@
   if (!CheckShift(S, OpPC, RHS, Bits))
 return false;
 
-  unsigned URHS = static_cast(RHS);
-  S.Stk.push(LT::from(static_cast(LHS) >> URHS, LHS.bitWidth()));
+  Integral R;
+  Integral::shiftRight(LHS.toUnsigned(), RHS, Bits, &R);
+  S.Stk.push(R);
   return true;
 }
 
@@ -1337,9 +1338,9 @@
   if (!CheckShift(S, OpPC, RHS, Bits))
 return false;
 
-  unsigned URHS = static_cast(RHS);
-  S.Stk.push(LT::from(static_cast(LHS) << URHS, LHS.bitWidth()));
-
+  Integral R;
+  Integral::shiftLeft(LHS.toUnsigned(), RHS, Bits, &R);
+  S.Stk.push(R);
   return true;
 }
 
Index: clang/lib/AST/Interp/Integral.h
===
--- clang/lib/AST/Interp/Integral.h
+++ clang/lib/AST/Interp/Integral.h
@@ -241,6 +241,18 @@
 return false;
   }
 
+  template 
+  static void shiftLeft(const Integral A, const Integral B,
+unsigned OpBits, Integral *R) {
+*R = Integral::from(A.V << B.V, OpBits);
+  }
+
+  template 
+  static void shiftRight(const Integral A, const Integral B,
+ unsigned OpBits, Integral *R) {
+*R = Integral::from(A.V >> B.V, OpBits);
+  }
+
 private:
   template  static bool CheckAddUB(T A, T B, T &R) {
 if constexpr (std::is_signed_v) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140339: [clang-format] Remove special logic for parsing concept definitions.

[Emilia Dreamer via Phabricator via cfe-commits]

rymiel added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:23946-23947
-  verifyFormat("template \n"
-   "concept C =\n"
-   "class X;");
 

owenpan wrote:
> Just want to make sure that the line break didn't matter.
Sorry, that line break was removed when verifyFormat was still used


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140339

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

[PATCH] D140395: [clang][analyzer] Extend StreamChecker with some new functions.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Does this patch fix any false positives from before, or is this just all new 
stuff? I ask, because I wonder whats the shortest path towards popping these 
checkers out of alpha, and fix what we already have. By no means am I saying 
that we should postpone landing this, but take a more directed attempt at tying 
off loose ends after this stack.




Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:952
 
+  // According to POSIX no change to 'errno' shall happen.
+

Why the comment? Seems like its solely `StdLibraryFunctionChecker`'s job to 
handle errno.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:979
 
+  // According to POSIX no change to 'errno' shall happen.
+

Same.



Comment at: clang/test/Analysis/stream-errno-note.c:1-2
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.unix.Stream,alpha.unix.Errno,apiModeling.StdCLibraryFunctions
 \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true 
-analyzer-output text -verify %s
+

Can you break this line up, such that we have an `-analyzer-checker=` argument 
for each package/checker?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140395

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

[PATCH] D137790: [clang][analyzer] Remove report of null stream from StreamChecker.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Are you sure that the refactoring made no changes to the results? Could you 
maybe just run a nightly or something like that to confirm?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137790

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

[PATCH] D139935: [NFC] [Doc] Fix example for AnnotateTypeDocs

[Xiang Li via Phabricator via cfe-commits]

python3kgae added a comment.

Happy new year and ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139935

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

[PATCH] D137205: [clang-tidy] Add performance-unnecessary-copy-on-last-use check

[Fabian Keßler via Phabricator via cfe-commits]

Febbe added a comment.

In D137205#4018548 , @ccotter wrote:

> @Febbe - Really glad to see this phab up! Not having realized this phab was 
> in progress, I implemented this a few months back (though, originally not as 
> a clang-tidy tool and never published) and thought it'd be worth sharing 
> notes. I recently turned my non-clang-tidy implementation into a clang-tidy 
> one here 
> .
>  A couple suggestions based on my experience writing a similar tool,

Hello @ccotter cool, you found this phab, and that you also had the same idea. 
I would like it, to collaborate with you. I just don't have a clue how to do 
this properly and clean via phabricator. What's your suggestion?

> - Should we detect "escaped" values, i.e., values who have their address 
> taken, and not apply a FixIt in these cases? See this test 
> .

Currently, this check does not care, if a value is taken by reference or 
pointer, not even when the value is taken in the same scope.
This is in fact an open Todo.
A rule of thumb would be: if the called function is analyzable, analyze it. 
Else, declare the move safety as unsecure and do warn, if the user is 
interested in it.

Generally I think, that fix it's should be always displayed, when they produce 
compilable code. But the move safety should be displayed in the error message.
Of course, it would be good to disable questionable fixups for automatic fix up 
runs. But I didn't heart of a solution to differ in a clang-tidy check, between 
automatic fixups and those used in an IDE.

> - I took a slightly more conservative approach in only moving values whose 
> `QualType.isTrivialType()` is true, which addresses the 
> `shared_ptr` problem. We can then apply FixIts for containers 
> (std::vector etc) and algebraic types (tuple/optional) of said types (and 
> containers of containers of ...). In general, the approach I was aiming for 
> was to define a classification of "safe to move types" (inferred from 
> isTrivialType, and/or using a list of safe types of in the tool configuration 
> as you have done). Containers/algebraic types of safe to move types, or smart 
> pointers of safe to move types (being careful with any custom allocator which 
> might lead to a similar problem as the scope guard case) are also safe to 
> move, etc. Having the tool be able to distinguish "safe to move" FixIts from 
> "potentially unsafe" (as a tool mode, potentially with "safe" as the default) 
> would ensure safe refactoring.

But moving trivial types are equal to a copy, aren't they? Therefore, I search 
for types where this is false. 
I also think, because clang-tidy does not claim to be 100% correct, in contrast 
to a compiler which must be, having such cases like `shared_ptr` is 
acceptable. Mostly such cases also have a huge smell (store mutex along with 
the data etc...).

> - This might be a nitpick, but in the diagnostic message `warning: Parameter 
> '...'`, "Parameter" typically means something more specific (the names given 
> to function variables, aka, function parameters). How about `warning: Value 
> '...'`

Thank you for the catch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137205

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

[PATCH] D140775: [clangd] Hover: show CalleeArgInfo for literals

[Tom Praschan via Phabricator via cfe-commits]

tom-anders added a comment.

In D140775#4021634 , @nridge wrote:

> Do you want to add a simple test case for a non-literal expression? Something 
> like hovering over the `+` in `2 + 3` should work.

Will do!

> Also, this is pre-existing, but I noticed that in a case like this:
>
>   void bar(int);
>   void func() {
> int x = 42;
> bar(x);
>   }
>
> the hover for `x` includes a line that just says `Passed` -- is that useful 
> in any way? Should we just omit the CalleeArgInfo a case like that?

Hmm, the thing is that if the function signature is `void bar(int&)`, then 
"Passed by reference" would be useful again, so we can't just add a `if 
(!CalleeArgInfo->Name.empty())` around the `OS << "Passed ";` Maybe we could 
instead display "Passed **by value**" if and only if the name is empty?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140775

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

[clang] 5674385 - [clang][dataflow] Remove unused includes

[Dmitri Gribenko via cfe-commits]

Author: Dmitri Gribenko
Date: 2023-01-02T23:34:24+01:00
New Revision: 5674385577f144db2131278449665994acff035c

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

LOG: [clang][dataflow] Remove unused includes

Added: 


Modified: 
clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp

Removed: 




diff  --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp 
b/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
index 42ee23c4be4f5..f73933f3db3d4 100644
--- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
@@ -4,18 +4,11 @@
 #include "clang/AST/Stmt.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
-#include "clang/Analysis/CFG.h"
-#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
-#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Lex/Lexer.h"
-#include "clang/Serialization/PCHContainerOperations.h"
-#include "clang/Tooling/ArgumentsAdjusters.h"
-#include "clang/Tooling/Tooling.h"
-#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"



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

[PATCH] D140857: Fix Fuchsia dyld in asan-ubsan variant

[Shai Barack via Phabricator via cfe-commits]

shayba created this revision.
shayba added reviewers: phosek, leonardchan.
Herald added a subscriber: abrachet.
Herald added a project: All.
shayba requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

The Clang Fuchsia driver sets -dynamic-linker to ld.so.1 or to a variant if 
needed.
Fix the variant lib path in asan-ubsan variant.

See also:
https://bugs.fuchsia.dev/p/fuchsia/issues/detail?id=118344


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140857

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp


Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -105,8 +105,12 @@
 
   if (!Args.hasArg(options::OPT_shared) && !Args.hasArg(options::OPT_r)) {
 std::string Dyld = D.DyldPrefix;
-if (SanArgs.needsAsanRt() && SanArgs.needsSharedRt())
+if (SanArgs.needsAsanRt() && !SanArgs.needsUbsanRt() &&
+SanArgs.needsSharedRt())
   Dyld += "asan/";
+if (SanArgs.needsAsanRt() && SanArgs.needsUbsanRt() &&
+SanArgs.needsSharedRt())
+  Dyld += "asan-ubsan/";
 if (SanArgs.needsHwasanRt() && SanArgs.needsSharedRt())
   Dyld += "hwasan/";
 if (SanArgs.needsTsanRt() && SanArgs.needsSharedRt())


Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -105,8 +105,12 @@
 
   if (!Args.hasArg(options::OPT_shared) && !Args.hasArg(options::OPT_r)) {
 std::string Dyld = D.DyldPrefix;
-if (SanArgs.needsAsanRt() && SanArgs.needsSharedRt())
+if (SanArgs.needsAsanRt() && !SanArgs.needsUbsanRt() &&
+SanArgs.needsSharedRt())
   Dyld += "asan/";
+if (SanArgs.needsAsanRt() && SanArgs.needsUbsanRt() &&
+SanArgs.needsSharedRt())
+  Dyld += "asan-ubsan/";
 if (SanArgs.needsHwasanRt() && SanArgs.needsSharedRt())
   Dyld += "hwasan/";
 if (SanArgs.needsTsanRt() && SanArgs.needsSharedRt())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 9cf4419 - [clang] Use std::optional instead of llvm::Optional (NFC)

[Kazu Hirata via cfe-commits]

Author: Kazu Hirata
Date: 2023-01-02T15:54:57-08:00
New Revision: 9cf4419e2451febf09acdf28c7d52ebf436d3a7e

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

LOG: [clang] Use std::optional instead of llvm::Optional (NFC)

This is part of an effort to migrate from llvm::Optional to
std::optional:

https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716

Added: 


Modified: 
clang/include/clang/APINotes/Types.h
clang/include/clang/Basic/CLWarnings.h
clang/include/clang/Basic/CustomizableOptional.h
clang/include/clang/Basic/TargetID.h
clang/include/clang/Support/RISCVVIntrinsicUtils.h
clang/lib/Basic/CLWarnings.cpp
clang/lib/Basic/TargetID.cpp
clang/lib/DirectoryWatcher/DirectoryScanner.cpp
clang/lib/DirectoryWatcher/DirectoryScanner.h
clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
clang/lib/Support/RISCVVIntrinsicUtils.cpp
clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
clang/unittests/libclang/LibclangTest.cpp
clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
clang/utils/TableGen/NeonEmitter.cpp
clang/utils/TableGen/RISCVVEmitter.cpp

Removed: 




diff  --git a/clang/include/clang/APINotes/Types.h 
b/clang/include/clang/APINotes/Types.h
index 8dfc3bf4f1e58..af5f05bc0d365 100644
--- a/clang/include/clang/APINotes/Types.h
+++ b/clang/include/clang/APINotes/Types.h
@@ -10,9 +10,9 @@
 #define LLVM_CLANG_APINOTES_TYPES_H
 
 #include "clang/Basic/Specifiers.h"
-#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
 #include 
+#include 
 #include 
 
 namespace llvm {
@@ -74,12 +74,12 @@ class CommonEntityInfo {
   : Unavailable(0), UnavailableInSwift(0), SwiftPrivateSpecified(0),
 SwiftPrivate(0) {}
 
-  llvm::Optional isSwiftPrivate() const {
-return SwiftPrivateSpecified ? llvm::Optional(SwiftPrivate)
+  std::optional isSwiftPrivate() const {
+return SwiftPrivateSpecified ? std::optional(SwiftPrivate)
  : std::nullopt;
   }
 
-  void setSwiftPrivate(llvm::Optional Private) {
+  void setSwiftPrivate(std::optional Private) {
 SwiftPrivateSpecified = Private.has_value();
 SwiftPrivate = Private.value_or(0);
   }
@@ -131,38 +131,38 @@ class CommonTypeInfo : public CommonEntityInfo {
   /// The Swift type to which a given type is bridged.
   ///
   /// Reflects the swift_bridge attribute.
-  llvm::Optional SwiftBridge;
+  std::optional SwiftBridge;
 
   /// The NS error domain for this type.
-  llvm::Optional NSErrorDomain;
+  std::optional NSErrorDomain;
 
 public:
   CommonTypeInfo() {}
 
-  const llvm::Optional &getSwiftBridge() const {
+  const std::optional &getSwiftBridge() const {
 return SwiftBridge;
   }
 
-  void setSwiftBridge(const llvm::Optional &SwiftType) {
+  void setSwiftBridge(const std::optional &SwiftType) {
 SwiftBridge = SwiftType;
   }
 
-  void setSwiftBridge(const llvm::Optional &SwiftType) {
+  void setSwiftBridge(const std::optional &SwiftType) {
 SwiftBridge = SwiftType
-  ? llvm::Optional(std::string(*SwiftType))
+  ? std::optional(std::string(*SwiftType))
   : std::nullopt;
   }
 
-  const llvm::Optional &getNSErrorDomain() const {
+  const std::optional &getNSErrorDomain() const {
 return NSErrorDomain;
   }
 
-  void setNSErrorDomain(const llvm::Optional &Domain) {
+  void setNSErrorDomain(const std::optional &Domain) {
 NSErrorDomain = Domain;
   }
 
-  void setNSErrorDomain(const llvm::Optional &Domain) {
-NSErrorDomain = Domain ? llvm::Optional(std::string(*Domain))
+  void setNSErrorDomain(const std::optional &Domain) {
+NSErrorDomain = Domain ? std::optional(std::string(*Domain))
: std::nullopt;
   }
 
@@ -221,9 +221,9 @@ class ObjCContextInfo : public CommonTypeInfo {
   ///
   /// Returns the default nullability, if implied, or std::nullopt if there is
   /// none.
-  llvm::Optional getDefaultNullability() const {
+  std::optional getDefaultNullability() const {
 return HasDefaultNullability
-   ? llvm::Optional(
+   ? std::optional(
  static_cast(DefaultNullability))
: std::nullopt;
   }
@@ -237,21 +237,21 @@ class ObjCContextInfo : public CommonTypeInfo {
   bool hasDesignatedInits() const { return HasDesignatedInits; }
   void setHasDesignatedInits(bool Value) { HasDesignatedInits = Value; }
 
-  llvm::Optional getSwiftImportAsNonGeneric() const {
+  std::optional getSwiftImportAsNonGeneric() const {
 return SwiftImportAsNonGenericSpecified
-   ? llvm::Optional(SwiftImportAsNonGeneric)
+   ? std::op

[clang-tools-extra] 2c675be - [clang-tools-extra] Use std::optional instead of llvm::Optional (NFC)

[Kazu Hirata via cfe-commits]

Author: Kazu Hirata
Date: 2023-01-02T16:17:40-08:00
New Revision: 2c675be9b232c1d0b5c55cbcb196e71036c681ea

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

LOG: [clang-tools-extra] Use std::optional instead of llvm::Optional (NFC)

This is part of an effort to migrate from llvm::Optional to
std::optional:

https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716

Added: 


Modified: 
clang-tools-extra/clang-tidy/ClangTidyOptions.h
clang-tools-extra/clang-tidy/ClangTidyProfiling.cpp
clang-tools-extra/clang-tidy/ClangTidyProfiling.h
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigProvider.cpp
clang-tools-extra/clangd/ConfigYAML.cpp
clang-tools-extra/clangd/FuzzyMatch.cpp
clang-tools-extra/clangd/FuzzyMatch.h
clang-tools-extra/clangd/PathMapping.cpp
clang-tools-extra/clangd/PathMapping.h
clang-tools-extra/clangd/TidyProvider.cpp
clang-tools-extra/clangd/index/dex/PostingList.cpp
clang-tools-extra/clangd/support/FileCache.cpp
clang-tools-extra/clangd/support/FileCache.h
clang-tools-extra/clangd/support/Threading.cpp
clang-tools-extra/clangd/support/Threading.h
clang-tools-extra/clangd/support/Trace.cpp
clang-tools-extra/clangd/unittests/FuzzyMatchTests.cpp
clang-tools-extra/clangd/unittests/LSPClient.h
clang-tools-extra/clangd/unittests/LoggerTests.cpp
clang-tools-extra/clangd/unittests/PathMappingTests.cpp
clang-tools-extra/clangd/unittests/support/CancellationTests.cpp
clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h
clang-tools-extra/pseudo/lib/grammar/Grammar.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.h 
b/clang-tools-extra/clang-tidy/ClangTidyOptions.h
index 6102399d9d387..727c55c3e 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyOptions.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.h
@@ -10,7 +10,6 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYOPTIONS_H
 
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
-#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ErrorOr.h"
@@ -232,7 +231,7 @@ class FileOptionsBaseProvider : public 
DefaultOptionsProvider {
 
   /// Try to read configuration files from \p Directory using registered
   /// \c ConfigHandlers.
-  llvm::Optional tryReadConfigFile(llvm::StringRef Directory);
+  std::optional tryReadConfigFile(llvm::StringRef Directory);
 
   llvm::StringMap CachedOptions;
   ClangTidyOptions OverrideOptions;

diff  --git a/clang-tools-extra/clang-tidy/ClangTidyProfiling.cpp 
b/clang-tools-extra/clang-tidy/ClangTidyProfiling.cpp
index 6c5d86a69821b..3a03dc033f802 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyProfiling.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyProfiling.cpp
@@ -11,6 +11,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 #include 
 #include 
 
@@ -74,7 +75,7 @@ void ClangTidyProfiling::storeProfileData() {
   printAsJSON(OS);
 }
 
-ClangTidyProfiling::ClangTidyProfiling(llvm::Optional Storage)
+ClangTidyProfiling::ClangTidyProfiling(std::optional Storage)
 : Storage(std::move(Storage)) {}
 
 ClangTidyProfiling::~ClangTidyProfiling() {

diff  --git a/clang-tools-extra/clang-tidy/ClangTidyProfiling.h 
b/clang-tools-extra/clang-tidy/ClangTidyProfiling.h
index 9487a34620e0a..bf21659a502fb 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyProfiling.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyProfiling.h
@@ -9,10 +9,10 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYPROFILING_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYPROFILING_H
 
-#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Support/Chrono.h"
 #include "llvm/Support/Timer.h"
+#include 
 #include 
 
 namespace llvm {
@@ -35,9 +35,9 @@ class ClangTidyProfiling {
   };
 
 private:
-  llvm::Optional TG;
+  std::optional TG;
 
-  llvm::Optional Storage;
+  std::optional Storage;
 
   void printUserFriendlyTable(llvm::raw_ostream &OS);
   void printAsJSON(llvm::raw_ostream &OS);
@@ -49,7 +49,7 @@ class ClangTidyProfiling {
 
   ClangTidyProfiling() = default;
 
-  ClangTidyProfiling(llvm::Optional Storage);
+  ClangTidyProfiling(std::optional Storage);
 
   ~ClangTidyProfiling();
 };

diff  --git a/clang-tools-extra/clangd/ConfigFragment.h 
b/clang-tools-extra/clangd/ConfigFragment.h
index 98923b516752b..bcd1a05b4a999 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -33,9 +33,9 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONFIGFRAGMENT_H
 
 #include "ConfigProvider.h"
-#include "llvm/ADT/Optional.h"
 #i

[PATCH] D140859: [clang][dataflow] Allow analyzing multiple functions in unit tests

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a project: All.
gribozavr requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

In unit tests for concrete dataflow analyses we typically use the
testonly `checkDataflow()` helper to analyse a free function called
"target". This pattern allows our tests to be uniform and focused on
specific statement- or expression-level C++ features.

As we expand our feature coverage, we want to analyze functions whose
names we don't fully control, like constructors, destructors, operators
etc. In such tests it is often convenient to analyze all functions
defined in the input code, to avoid having to carefully craft an AST
matcher that finds the exact function we're interested in. That can be
easily done by providing `checkDataflow()` with a catch-all matcher like
`functionDecl()`.

It is also often convenient to define multiple special member functions
in a single unit test, for example, multiple constructors, and share the
rest of the class definition code between constructors. As a result, it
makes sense to analyze multiple functions in one unit test.

This change allows `checkDataflow()` to correctly handle AST matchers
that match more than one function. Previously, it would only ever
analyze the first matched function, and silently ignore the rest. Now it
runs dataflow analysis in a loop, and calls `VerifyResults` for each
function that was found in the input and analyzed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140859

Files:
  clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TestingSupportTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TestingSupportTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupportTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TestingSupportTest.cpp
@@ -15,6 +15,7 @@
 namespace {
 
 using ::clang::ast_matchers::functionDecl;
+using ::clang::ast_matchers::hasAnyName;
 using ::clang::ast_matchers::hasName;
 using ::clang::ast_matchers::isDefinition;
 using ::clang::dataflow::test::AnalysisInputs;
@@ -74,21 +75,21 @@
 }
 
 void checkDataflow(
-llvm::StringRef Code, llvm::StringRef Target,
+llvm::StringRef Code,
+ast_matchers::internal::Matcher TargetFuncMatcher,
 std::function<
 void(const llvm::StringMap> &,
  const AnalysisOutputs &)>
 Expectations) {
   ASSERT_THAT_ERROR(checkDataflow(
 AnalysisInputs(
-Code, hasName(Target),
+Code, std::move(TargetFuncMatcher),
 [](ASTContext &Context, Environment &) {
   return NoopAnalysis(
   Context, /*ApplyBuiltinTransfer=*/false);
 })
 .withASTBuildArgs({"-fsyntax-only", "-std=c++17"}),
-/*VerifyResults=*/
-std::move(Expectations)),
+/*VerifyResults=*/std::move(Expectations)),
 llvm::Succeeded());
 }
 
@@ -100,7 +101,8 @@
 
   EXPECT_CALL(Expectations, Call(IsEmpty(), _)).Times(1);
 
-  checkDataflow("void target() {}", "target", Expectations.AsStdFunction());
+  checkDataflow("void target() {}", hasName("target"),
+Expectations.AsStdFunction());
 }
 
 TEST(ProgramPointAnnotations, NoAnnotationsDifferentTarget) {
@@ -111,10 +113,11 @@
 
   EXPECT_CALL(Expectations, Call(IsEmpty(), _)).Times(1);
 
-  checkDataflow("void fun() {}", "fun", Expectations.AsStdFunction());
+  checkDataflow("void target() {}", hasName("target"),
+Expectations.AsStdFunction());
 }
 
-TEST(ProgramPointAnnotations, WithCodepoint) {
+TEST(ProgramPointAnnotations, WithProgramPoint) {
   ::testing::MockFunction> &,
   const AnalysisOutputs &)>
@@ -126,13 +129,13 @@
   .Times(1);
 
   checkDataflow(R"cc(void target() {
- int n;
- // [[program-point]]
-   })cc",
-"target", Expectations.AsStdFunction());
+   int n;
+   // [[program-point]]
+ })cc",
+hasName("target"), Expectations.AsStdFunction());
 }
 
-TEST(ProgramPointAnnotations, MultipleCodepoints) {
+TEST(ProgramPointAnnotations, MultipleProgramPoints) {
   ::testing::MockFunction> &,
   const AnalysisOutputs &)>
@@ -145,15 +148,59 @@
   .Times(1);
 
   checkDataflow(R"cc(void target(bool b) {
- if (b) {
-   int n;
-   // [[program-point-1]]
- } else {
-   int m;
-   // [[program-point-2]]
- 

[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

[Takuya Shimizu via Phabricator via cfe-commits]

hazohelet created this revision.
hazohelet added a project: clang.
Herald added a project: All.
hazohelet requested review of this revision.

When using the `&&` operator within a `||` operator, both Clang and GCC produce 
a warning for potentially confusing operator precedence. However, Clang avoids 
this warning for certain patterns, such as `a && b || 0` or `a || b && 1`, 
where the operator precedence of `&&` and `||` does not change the result.

However, this behavior appears inconsistent when using the `const` or 
`constexpr` qualifiers. For example:

  bool t = true;
  bool tt = true || false && t; // Warning: '&&' within '||' 
[-Wlogical-op-parentheses]

  const bool t = true;
  bool tt = true || false && t; // No warning

  const bool t = false;
  bool tt = true || false && t; // Warning: '&&' within '||' 
[-Wlogical-op-parentheses]

The second example does not produce a warning because `true || false && t` 
matches the `a || b && 1` pattern, while the third one does not match any of 
them.

This behavior can lead to the lack of warnings for complicated `constexpr` 
expressions. Clang should only suppress this warning when literal values are 
placed in the place of `t` in the examples above.

This patch adds the literal-or-not check to fix the inconsistent warnings for 
`&&` within `||` when using const or constexpr.


https://reviews.llvm.org/D140860

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/logical-op-parentheses.c
  clang/test/Sema/logical-op-parentheses.cpp

Index: clang/test/Sema/logical-op-parentheses.cpp
===
--- /dev/null
+++ clang/test/Sema/logical-op-parentheses.cpp
@@ -0,0 +1,66 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DSILENCE
+// RUN: %clang_cc1 -fsyntax-only -verify %s -Wlogical-op-parentheses
+// RUN: %clang_cc1 -fsyntax-only -verify %s -Wparentheses
+// RUN: %clang_cc1 -fsyntax-only -fdiagnostics-parseable-fixits %s -Wlogical-op-parentheses 2>&1 | FileCheck %s
+
+#ifdef SILENCE
+// expected-no-diagnostics
+#endif
+
+void logical_op_parentheses(unsigned i) {
+	constexpr bool t = 1;
+  (void)(i ||
+ i && i);
+#ifndef SILENCE
+  // expected-warning@-2 {{'&&' within '||'}}
+  // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
+#endif
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:14-[[@LINE-5]]:14}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:20-[[@LINE-6]]:20}:")"
+
+  static_assert(t ||
+ t && t);
+#ifndef SILENCE
+  // expected-warning@-2 {{'&&' within '||'}}
+  // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
+#endif
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:14-[[@LINE-5]]:14}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:20-[[@LINE-6]]:20}:")"
+
+  static_assert(t && 
+ t || t);
+#ifndef SILENCE
+  // expected-warning@-3 {{'&&' within '||'}}
+  // expected-note@-4 {{place parentheses around the '&&' expression to silence this warning}}
+#endif
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:17-[[@LINE-6]]:17}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:15-[[@LINE-6]]:15}:")"
+	
+	(void)(i || i && "w00t");
+  (void)("w00t" && i || i);
+  (void)("w00t" && t || t);
+  static_assert(t && t || 0);
+  static_assert(1 && t || t);
+  static_assert(0 || t && t);
+  static_assert(t || t && 1);
+
+  (void)(i || i && "w00t" || i);
+#ifndef SILENCE
+  // expected-warning@-2 {{'&&' within '||'}}
+  // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
+#endif
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:15-[[@LINE-5]]:15}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:26-[[@LINE-6]]:26}:")"
+
+  (void)(i || "w00t" && i || i);
+#ifndef SILENCE
+  // expected-warning@-2 {{'&&' within '||'}}
+  // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
+#endif
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:15-[[@LINE-5]]:15}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:26-[[@LINE-6]]:26}:")"
+
+  (void)(i && i || 0);
+  (void)(0 || i && i);
+}
+
Index: clang/test/Sema/logical-op-parentheses.c
===
--- clang/test/Sema/logical-op-parentheses.c
+++ clang/test/Sema/logical-op-parentheses.c
@@ -8,6 +8,7 @@
 #endif
 
 void logical_op_parentheses(unsigned i) {
+	const unsigned t = 1;
   (void)(i ||
  i && i);
 #ifndef SILENCE
@@ -17,8 +18,31 @@
   // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:14-[[@LINE-5]]:14}:"("
   // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:20-[[@LINE-6]]:20}:")"
 
-  (void)(i || i && "w00t");
+  (void)(t ||
+ t && t);
+#ifndef SILENCE
+  // expected-warning@-2 {{'&&' within '||'}}
+  // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
+#endif
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:14-[[@LINE-5]]:14}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:20-[[@LINE-6]]:20}:")"
+
+  (v

[PATCH] D140270: MIPS: fix build from IR files, nan2008 and FpAbi

[YunQiang Su via Phabricator via cfe-commits]

wzssyqa updated this revision to Diff 485897.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140270

Files:
  clang/lib/Driver/ToolChains/Arch/Mips.cpp
  clang/test/Driver/mips-as.c
  clang/test/Driver/mips-integrated-as.s
  llvm/lib/Target/Mips/MipsAsmPrinter.cpp
  llvm/test/CodeGen/Mips/abiflags-2008-fp64.ll

Index: llvm/test/CodeGen/Mips/abiflags-2008-fp64.ll
===
--- /dev/null
+++ llvm/test/CodeGen/Mips/abiflags-2008-fp64.ll
@@ -0,0 +1,13 @@
+; RUN: llc %s -o - | FileCheck %s
+
+target triple = "mipsel-unknown-linux-gnu"
+
+define dso_local void @test() #0 {
+  ret void
+}
+
+attributes #0 = { "target-cpu"="mips32r2" "target-features"="+fp64,+mips32r2,+nan2008" }
+
+
+; CHECK: .nan2008
+; CHECK: .module fp=64
Index: llvm/lib/Target/Mips/MipsAsmPrinter.cpp
===
--- llvm/lib/Target/Mips/MipsAsmPrinter.cpp
+++ llvm/lib/Target/Mips/MipsAsmPrinter.cpp
@@ -777,14 +777,18 @@
   // around it by re-initializing the PIC state here.
   TS.setPic(OutContext.getObjectFileInfo()->isPositionIndependent());
 
+  // Try to get target-features from the first function.
+  StringRef FS = TM.getTargetFeatureString();
+  Module::iterator F = M.begin();
+  if (FS.empty() && M.size() && F->hasFnAttribute("target-features"))
+FS = F->getFnAttribute("target-features").getValueAsString();
+
   // Compute MIPS architecture attributes based on the default subtarget
-  // that we'd have constructed. Module level directives aren't LTO
-  // clean anyhow.
+  // that we'd have constructed.
   // FIXME: For ifunc related functions we could iterate over and look
   // for a feature string that doesn't match the default one.
   const Triple &TT = TM.getTargetTriple();
   StringRef CPU = MIPS_MC::selectMipsCPU(TT, TM.getTargetCPU());
-  StringRef FS = TM.getTargetFeatureString();
   const MipsTargetMachine &MTM = static_cast(TM);
   const MipsSubtarget STI(TT, CPU, FS, MTM.isLittleEndian(), MTM, std::nullopt);
 
Index: clang/test/Driver/mips-integrated-as.s
===
--- clang/test/Driver/mips-integrated-as.s
+++ clang/test/Driver/mips-integrated-as.s
@@ -160,8 +160,8 @@
 // RUN: %clang -target mips-linux-gnu -### -fintegrated-as -c %s 2>&1 | \
 // RUN:   FileCheck -check-prefix=FPXX-DEFAULT %s
 // FPXX-DEFAULT: -cc1as
-// FPXX-DEFAULT-NOT: "-target-feature" "+fpxx"
-// FPXX-DEFAULT-NOT: "-target-feature" "+nooddspreg"
+// FPXX-DEFAULT: "-target-feature" "+fpxx"
+// FPXX-DEFAULT: "-target-feature" "+nooddspreg"
 
 // RUN: %clang -target mips-linux-gnu -### -fintegrated-as -c %s -mfp32 2>&1 | \
 // RUN:   FileCheck -check-prefix=FP32 %s
@@ -182,7 +182,7 @@
 // RUN: %clang -target mips-linux-gnu -### -fintegrated-as -c %s 2>&1 | \
 // RUN:   FileCheck -check-prefix=ODDSPREG-DEFAULT %s
 // ODDSPREG-DEFAULT: -cc1as
-// ODDSPREG-DEFAULT-NOT: "-target-feature" "{{[+-]}}nooddspreg"
+// ODDSPREG-DEFAULT: "-target-feature" "+nooddspreg"
 
 // RUN: %clang -target mips-linux-gnu -### -fintegrated-as -c %s -modd-spreg 2>&1 | \
 // RUN:   FileCheck -check-prefix=ODDSPREG-ON %s
Index: clang/test/Driver/mips-as.c
===
--- clang/test/Driver/mips-as.c
+++ clang/test/Driver/mips-as.c
@@ -196,7 +196,7 @@
 // RUN: %clang -target mips-linux-gnu -mno-mips16 -mips16 -### \
 // RUN:   -no-integrated-as -fno-pic -c %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=MIPS-16 %s
-// MIPS-16: as{{(.exe)?}}" "-march" "mips32r2" "-mabi" "32" "-mno-shared" "-call_nonpic" "-EB" "-mips16"
+// MIPS-16: as{{(.exe)?}}" "-march" "mips32r2" "-mabi" "32" "-mno-shared" "-call_nonpic" "-EB" "-mfpxx" "-mips16"
 //
 // RUN: %clang -target mips-linux-gnu -mips16 -mno-mips16 -### \
 // RUN:   -no-integrated-as -fno-pic -c %s 2>&1 \
@@ -207,7 +207,7 @@
 // RUN: %clang -target mips-linux-gnu -mno-micromips -mmicromips -### \
 // RUN:   -no-integrated-as -fno-pic -c %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=MIPS-MICRO %s
-// MIPS-MICRO: as{{(.exe)?}}" "-march" "mips32r2" "-mabi" "32" "-mno-shared" "-call_nonpic" "-EB" "-mmicromips"
+// MIPS-MICRO: as{{(.exe)?}}" "-march" "mips32r2" "-mabi" "32" "-mno-shared" "-call_nonpic" "-EB" "-mfpxx" "-mmicromips"
 //
 // RUN: %clang -target mips-linux-gnu -mmicromips -mno-micromips -### \
 // RUN:   -no-integrated-as -fno-pic -c %s 2>&1 \
@@ -218,7 +218,7 @@
 // RUN: %clang -target mips-linux-gnu -mno-dsp -mdsp -### \
 // RUN:   -no-integrated-as -fno-pic -c %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=MIPS-DSP %s
-// MIPS-DSP: as{{(.exe)?}}" "-march" "mips32r2" "-mabi" "32" "-mno-shared" "-call_nonpic" "-EB" "-mdsp"
+// MIPS-DSP: as{{(.exe)?}}" "-march" "mips32r2" "-mabi" "32" "-mno-shared" "-call_nonpic" "-EB" "-mfpxx" "-mdsp"
 //
 // RUN: %clang -target mips-linux-gnu -mdsp -mno-dsp -### \
 // RUN:   -no-int

[PATCH] D140270: MIPS: fix build from IR files, nan2008 and FpAbi

[YunQiang Su via Phabricator via cfe-commits]

wzssyqa updated this revision to Diff 485898.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140270

Files:
  clang/lib/Driver/ToolChains/Arch/Mips.cpp
  clang/test/Driver/mips-as.c
  clang/test/Driver/mips-integrated-as.s
  llvm/lib/Target/Mips/MipsAsmPrinter.cpp
  llvm/test/CodeGen/Mips/abiflags-2008-fp64.ll

Index: llvm/test/CodeGen/Mips/abiflags-2008-fp64.ll
===
--- /dev/null
+++ llvm/test/CodeGen/Mips/abiflags-2008-fp64.ll
@@ -0,0 +1,18 @@
+;; When we compile object with "-flto", the info of "-mnan=2008"
+;; and "-mfp32/-mfpxx/-mfp64" will be missing in the result IR file.
+;; Thus the asm/obj files will have wrong format.
+;; With D140270 we extract these info from the first function,
+;; and set it for the whole compile unit.
+; RUN: llc %s -o - | FileCheck %s
+
+target triple = "mipsel-unknown-linux-gnu"
+
+define dso_local void @test() #0 {
+  ret void
+}
+
+attributes #0 = { "target-cpu"="mips32r2" "target-features"="+fp64,+mips32r2,+nan2008" }
+
+
+; CHECK: .nan2008
+; CHECK: .module fp=64
Index: llvm/lib/Target/Mips/MipsAsmPrinter.cpp
===
--- llvm/lib/Target/Mips/MipsAsmPrinter.cpp
+++ llvm/lib/Target/Mips/MipsAsmPrinter.cpp
@@ -777,14 +777,18 @@
   // around it by re-initializing the PIC state here.
   TS.setPic(OutContext.getObjectFileInfo()->isPositionIndependent());
 
+  // Try to get target-features from the first function.
+  StringRef FS = TM.getTargetFeatureString();
+  Module::iterator F = M.begin();
+  if (FS.empty() && M.size() && F->hasFnAttribute("target-features"))
+FS = F->getFnAttribute("target-features").getValueAsString();
+
   // Compute MIPS architecture attributes based on the default subtarget
-  // that we'd have constructed. Module level directives aren't LTO
-  // clean anyhow.
+  // that we'd have constructed.
   // FIXME: For ifunc related functions we could iterate over and look
   // for a feature string that doesn't match the default one.
   const Triple &TT = TM.getTargetTriple();
   StringRef CPU = MIPS_MC::selectMipsCPU(TT, TM.getTargetCPU());
-  StringRef FS = TM.getTargetFeatureString();
   const MipsTargetMachine &MTM = static_cast(TM);
   const MipsSubtarget STI(TT, CPU, FS, MTM.isLittleEndian(), MTM, std::nullopt);
 
Index: clang/test/Driver/mips-integrated-as.s
===
--- clang/test/Driver/mips-integrated-as.s
+++ clang/test/Driver/mips-integrated-as.s
@@ -160,8 +160,8 @@
 // RUN: %clang -target mips-linux-gnu -### -fintegrated-as -c %s 2>&1 | \
 // RUN:   FileCheck -check-prefix=FPXX-DEFAULT %s
 // FPXX-DEFAULT: -cc1as
-// FPXX-DEFAULT-NOT: "-target-feature" "+fpxx"
-// FPXX-DEFAULT-NOT: "-target-feature" "+nooddspreg"
+// FPXX-DEFAULT: "-target-feature" "+fpxx"
+// FPXX-DEFAULT: "-target-feature" "+nooddspreg"
 
 // RUN: %clang -target mips-linux-gnu -### -fintegrated-as -c %s -mfp32 2>&1 | \
 // RUN:   FileCheck -check-prefix=FP32 %s
@@ -182,7 +182,7 @@
 // RUN: %clang -target mips-linux-gnu -### -fintegrated-as -c %s 2>&1 | \
 // RUN:   FileCheck -check-prefix=ODDSPREG-DEFAULT %s
 // ODDSPREG-DEFAULT: -cc1as
-// ODDSPREG-DEFAULT-NOT: "-target-feature" "{{[+-]}}nooddspreg"
+// ODDSPREG-DEFAULT: "-target-feature" "+nooddspreg"
 
 // RUN: %clang -target mips-linux-gnu -### -fintegrated-as -c %s -modd-spreg 2>&1 | \
 // RUN:   FileCheck -check-prefix=ODDSPREG-ON %s
Index: clang/test/Driver/mips-as.c
===
--- clang/test/Driver/mips-as.c
+++ clang/test/Driver/mips-as.c
@@ -196,7 +196,7 @@
 // RUN: %clang -target mips-linux-gnu -mno-mips16 -mips16 -### \
 // RUN:   -no-integrated-as -fno-pic -c %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=MIPS-16 %s
-// MIPS-16: as{{(.exe)?}}" "-march" "mips32r2" "-mabi" "32" "-mno-shared" "-call_nonpic" "-EB" "-mips16"
+// MIPS-16: as{{(.exe)?}}" "-march" "mips32r2" "-mabi" "32" "-mno-shared" "-call_nonpic" "-EB" "-mfpxx" "-mips16"
 //
 // RUN: %clang -target mips-linux-gnu -mips16 -mno-mips16 -### \
 // RUN:   -no-integrated-as -fno-pic -c %s 2>&1 \
@@ -207,7 +207,7 @@
 // RUN: %clang -target mips-linux-gnu -mno-micromips -mmicromips -### \
 // RUN:   -no-integrated-as -fno-pic -c %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=MIPS-MICRO %s
-// MIPS-MICRO: as{{(.exe)?}}" "-march" "mips32r2" "-mabi" "32" "-mno-shared" "-call_nonpic" "-EB" "-mmicromips"
+// MIPS-MICRO: as{{(.exe)?}}" "-march" "mips32r2" "-mabi" "32" "-mno-shared" "-call_nonpic" "-EB" "-mfpxx" "-mmicromips"
 //
 // RUN: %clang -target mips-linux-gnu -mmicromips -mno-micromips -### \
 // RUN:   -no-integrated-as -fno-pic -c %s 2>&1 \
@@ -218,7 +218,7 @@
 // RUN: %clang -target mips-linux-gnu -mno-dsp -mdsp -### \
 // RUN:   -no-integrated-as -fno-pic -c %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=MIPS-DSP %s
-// MIPS

[clang] f2bec87 - [NFC][Clang][RISCV] Reduce boilerplate when determining prototype for segment loads

[via cfe-commits]

Author: eopXD
Date: 2023-01-02T18:50:36-08:00
New Revision: f2bec8702688ea034cefcf4e13922aa4d40bd4a0

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

LOG: [NFC][Clang][RISCV] Reduce boilerplate when determining prototype for 
segment loads

No functionality change of the RVV builtin and compiler intrinsics is intended
in this patch.

This patch gathers scattered comments for the segment load builtin/intrinsics
and its variants (e.g. segment unit-stride load, segment strided load) into a
single paragraph under riscv_vector.td.

This patch also tries to reduce one level of the if-statements as the push_back
are essentially the same actions but differs in index based on the the value
of the policy attributes and whether the intrinsic is masked.

Reviewed By: kito-cheng

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

Added: 


Modified: 
clang/include/clang/Basic/riscv_vector.td

Removed: 




diff  --git a/clang/include/clang/Basic/riscv_vector.td 
b/clang/include/clang/Basic/riscv_vector.td
index 9c05d893dde2..6e089a7d87d6 100644
--- a/clang/include/clang/Basic/riscv_vector.td
+++ b/clang/include/clang/Basic/riscv_vector.td
@@ -812,6 +812,80 @@ multiclass RVVIndexedStore {
 }
 
 defvar NFList = [2, 3, 4, 5, 6, 7, 8];
+/*
+A segment load builtin has 
diff erent variants.
+
+Therefore a segment unit-stride load builtin can have 4 variants,
+1. When unmasked and the policies are all specified as agnostic:
+(Address0, ..., Address{NF - 1}, Ptr, VL)
+2. When masked and the policies are all specified as agnostic:
+(Address0, ..., Address{NF - 1}, Mask, Ptr, VL)
+3. When unmasked and one of the policies is specified as undisturbed:
+(Address0, ..., Address{NF - 1}, Maskedoff0, ..., Maskedoff{NF - 1},
+  Ptr, VL)
+4. When masked and one of the policies is specified as undisturbed:
+(Address0, ..., Address{NF - 1}, Mask, Maskedoff0, ..., Maskedoff{NF - 1},
+  Ptr, VL)
+
+Other variants of segment load builtin share the same structure, but they
+have their own extra parameter.
+
+The segment unit-stride fault-only-first load builtin has a 'NewVL'
+operand after the 'Ptr' operand.
+1. When unmasked and the policies are all specified as agnostic:
+(Address0, ..., Address{NF - 1}, Ptr, NewVL, VL)
+2. When masked and the policies are all specified as agnostic:
+(Address0, ..., Address{NF - 1}, Mask, Ptr, NewVL, VL)
+3. When unmasked and one of the policies is specified as undisturbed:
+(Address0, ..., Address{NF - 1}, Maskedoff0, ..., Maskedoff{NF - 1},
+  Ptr, NewVL, VL)
+4. When masked and one of the policies is specified as undisturbed:
+(Address0, ..., Address{NF - 1}, Mask, Maskedoff0, ..., Maskedoff{NF - 1},
+  Ptr, NewVL, VL)
+
+The segment strided load builtin has a 'Stride' operand after the 'Ptr'
+operand.
+1. When unmasked and the policies are all specified as agnostic:
+(Address0, ..., Address{NF - 1}, Ptr, Stride, VL)
+2. When masked and the policies are all specified as agnostic:
+(Address0, ..., Address{NF - 1}, Mask, Ptr, Stride, VL)
+3. When unmasked and one of the policies is specified as undisturbed:
+(Address0, ..., Address{NF - 1}, Maskedoff0, ..., Maskedoff{NF - 1},
+  Ptr, Stride, VL)
+4. When masked and one of the policies is specified as undisturbed:
+(Address0, ..., Address{NF - 1}, Mask, Maskedoff0, ..., Maskedoff{NF - 1},
+  Ptr, Stride, VL)
+
+The segment indexed load builtin has a 'Idx' operand after the 'Ptr' operand.
+1. When unmasked and the policies are all specified as agnostic:
+(Address0, ..., Address{NF - 1}, Ptr, Idx, VL)
+2. When masked and the policies are all specified as agnostic:
+(Address0, ..., Address{NF - 1}, Mask, Ptr, Idx, VL)
+3. When unmasked and one of the policies is specified as undisturbed:
+(Address0, ..., Address{NF - 1}, Maskedoff0, ..., Maskedoff{NF - 1},
+  Ptr, Idx, VL)
+4. When masked and one of the policies is specified as undisturbed:
+(Address0, ..., Address{NF - 1}, Mask, Maskedoff0, ..., Maskedoff{NF - 1},
+  Ptr, Idx, VL)
+
+Segment load intrinsics has 
diff erent variants similar to their builtins.
+
+Segment unit-stride load intrinsic,
+  Masked: (Vector0, ..., Vector{NF - 1}, Ptr, Mask, VL, Policy)
+  Unmasked: (Vector0, ..., Vector{NF - 1}, Ptr, VL)
+Segment unit-stride fault-only-first load intrinsic,
+  Masked: (Vector0, ..., Vector{NF - 1}, Ptr, Mask, VL, Policy)
+  Unmasked: (Vector0, ..., Vector{NF - 1}, Ptr, VL)
+Segment strided load intrinsic,
+  Masked: (Vector0, ..., Vector{NF - 1}, Ptr, Stride, Mask, VL, Policy)
+  Unmasked: (Vector0, ..., Vector{NF - 1}, Ptr, Stride, VL)
+Segment indexed load intrinsic,
+  Masked: (Vector0, ..., Vector{NF - 1}, Ptr, Index, Mask, VL, Policy)
+  Unmasked: (Vector0, ..., Vector{NF - 1}, Ptr, Index, VL)
+
+The Vector(s) is poison when the policy behavior allows us to not

[PATCH] D140662: [NFC][Clang][RISCV] Reduce boilerplate when determining prototype for segment loads

[Yueh-Ting (eop) Chen 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 rGf2bec8702688: [NFC][Clang][RISCV] Reduce boilerplate when 
determining prototype for segment… (authored by eopXD).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140662

Files:
  clang/include/clang/Basic/riscv_vector.td

Index: clang/include/clang/Basic/riscv_vector.td
===
--- clang/include/clang/Basic/riscv_vector.td
+++ clang/include/clang/Basic/riscv_vector.td
@@ -812,6 +812,80 @@
 }
 
 defvar NFList = [2, 3, 4, 5, 6, 7, 8];
+/*
+A segment load builtin has different variants.
+
+Therefore a segment unit-stride load builtin can have 4 variants,
+1. When unmasked and the policies are all specified as agnostic:
+(Address0, ..., Address{NF - 1}, Ptr, VL)
+2. When masked and the policies are all specified as agnostic:
+(Address0, ..., Address{NF - 1}, Mask, Ptr, VL)
+3. When unmasked and one of the policies is specified as undisturbed:
+(Address0, ..., Address{NF - 1}, Maskedoff0, ..., Maskedoff{NF - 1},
+  Ptr, VL)
+4. When masked and one of the policies is specified as undisturbed:
+(Address0, ..., Address{NF - 1}, Mask, Maskedoff0, ..., Maskedoff{NF - 1},
+  Ptr, VL)
+
+Other variants of segment load builtin share the same structure, but they
+have their own extra parameter.
+
+The segment unit-stride fault-only-first load builtin has a 'NewVL'
+operand after the 'Ptr' operand.
+1. When unmasked and the policies are all specified as agnostic:
+(Address0, ..., Address{NF - 1}, Ptr, NewVL, VL)
+2. When masked and the policies are all specified as agnostic:
+(Address0, ..., Address{NF - 1}, Mask, Ptr, NewVL, VL)
+3. When unmasked and one of the policies is specified as undisturbed:
+(Address0, ..., Address{NF - 1}, Maskedoff0, ..., Maskedoff{NF - 1},
+  Ptr, NewVL, VL)
+4. When masked and one of the policies is specified as undisturbed:
+(Address0, ..., Address{NF - 1}, Mask, Maskedoff0, ..., Maskedoff{NF - 1},
+  Ptr, NewVL, VL)
+
+The segment strided load builtin has a 'Stride' operand after the 'Ptr'
+operand.
+1. When unmasked and the policies are all specified as agnostic:
+(Address0, ..., Address{NF - 1}, Ptr, Stride, VL)
+2. When masked and the policies are all specified as agnostic:
+(Address0, ..., Address{NF - 1}, Mask, Ptr, Stride, VL)
+3. When unmasked and one of the policies is specified as undisturbed:
+(Address0, ..., Address{NF - 1}, Maskedoff0, ..., Maskedoff{NF - 1},
+  Ptr, Stride, VL)
+4. When masked and one of the policies is specified as undisturbed:
+(Address0, ..., Address{NF - 1}, Mask, Maskedoff0, ..., Maskedoff{NF - 1},
+  Ptr, Stride, VL)
+
+The segment indexed load builtin has a 'Idx' operand after the 'Ptr' operand.
+1. When unmasked and the policies are all specified as agnostic:
+(Address0, ..., Address{NF - 1}, Ptr, Idx, VL)
+2. When masked and the policies are all specified as agnostic:
+(Address0, ..., Address{NF - 1}, Mask, Ptr, Idx, VL)
+3. When unmasked and one of the policies is specified as undisturbed:
+(Address0, ..., Address{NF - 1}, Maskedoff0, ..., Maskedoff{NF - 1},
+  Ptr, Idx, VL)
+4. When masked and one of the policies is specified as undisturbed:
+(Address0, ..., Address{NF - 1}, Mask, Maskedoff0, ..., Maskedoff{NF - 1},
+  Ptr, Idx, VL)
+
+Segment load intrinsics has different variants similar to their builtins.
+
+Segment unit-stride load intrinsic,
+  Masked: (Vector0, ..., Vector{NF - 1}, Ptr, Mask, VL, Policy)
+  Unmasked: (Vector0, ..., Vector{NF - 1}, Ptr, VL)
+Segment unit-stride fault-only-first load intrinsic,
+  Masked: (Vector0, ..., Vector{NF - 1}, Ptr, Mask, VL, Policy)
+  Unmasked: (Vector0, ..., Vector{NF - 1}, Ptr, VL)
+Segment strided load intrinsic,
+  Masked: (Vector0, ..., Vector{NF - 1}, Ptr, Stride, Mask, VL, Policy)
+  Unmasked: (Vector0, ..., Vector{NF - 1}, Ptr, Stride, VL)
+Segment indexed load intrinsic,
+  Masked: (Vector0, ..., Vector{NF - 1}, Ptr, Index, Mask, VL, Policy)
+  Unmasked: (Vector0, ..., Vector{NF - 1}, Ptr, Index, VL)
+
+The Vector(s) is poison when the policy behavior allows us to not care
+about any masked-off elements.
+*/
 
 class PVString {
   string S =
@@ -842,36 +916,30 @@
 {
   ResultType = ConvertType(E->getArg(0)->getType()->getPointeeType());
   IntrinsicTypes = {ResultType, Ops.back()->getType()};
-  SmallVector Operands;
-  if (IsMasked) {
-// TAMA builtin: (val0 address, ..., mask, ptr, vl)
-// builtin: (val0 address, ..., mask, maskedoff0, ..., ptr, vl)
-// intrinsic: (maskedoff0, ..., ptr, mask, vl)
-if (PolicyAttrs == TAIL_AGNOSTIC_MASK_AGNOSTIC) {
-  Operands.append(NF, llvm::PoisonValue::get(ResultType));
-  Operands.push_back(Ops[NF + 1]);
-  Operands.push_back(Ops[NF]);
-  Operands.push_back(Ops[NF + 2]);
-} e

[clang] fbc8697 - [clang-format][NFC] Remove a superfluous semicolon after \version

[Owen Pan via cfe-commits]

Author: Owen Pan
Date: 2023-01-02T18:55:30-08:00
New Revision: fbc86973b58d7fe735596dfbe38884bc9398e52c

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

LOG: [clang-format][NFC] Remove a superfluous semicolon after \version

Added: 


Modified: 
clang/include/clang/Format/Format.h

Removed: 




diff  --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 514ef16d8708a..b6fd6611fb5b1 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -2867,7 +2867,7 @@ struct FormatStyle {
   };
 
   /// The pack constructor initializers style to use.
-  /// \version 14;
+  /// \version 14
   PackConstructorInitializersStyle PackConstructorInitializers;
 
   /// The penalty for breaking around an assignment operator.



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

[PATCH] D140868: [C] Make (c ? e1 : e2) noreturn only if both e1 and e2 are noreturn

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: aaron.ballman, mizvekov, rjmccall.
Herald added a subscriber: StephenFan.
Herald added a project: All.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

In C mode, if e1 is noreturn but e2 isn't, `(c ? e1 : e2)` is
incorrectly noreturn and Clang codegen produces `unreachable` which may
lead to miscompiles (see [1] `gawk/support/dfa.c`).
This problem has been known since
8c6b56f39d967347f28dd9c93f1cffddf6d7e4cd (2010) or earlier.

Fix this by making the result type noreturn only if both e1 and e2 are
noreturn.

Fix https://github.com/llvm/llvm-project/issues/59792 [1]


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140868

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CodeGen/attr-noreturn.c

Index: clang/test/CodeGen/attr-noreturn.c
===
--- clang/test/CodeGen/attr-noreturn.c
+++ clang/test/CodeGen/attr-noreturn.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -S -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64 -x c++ -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-CXX
 
 typedef void (*fptrs_t[4])(void);
 fptrs_t p __attribute__((noreturn));
@@ -8,3 +9,24 @@
 }
 // CHECK: call void
 // CHECK-NEXT: unreachable
+
+// CHECK-LABEL: @test_conditional(
+// CHECK: %cond = select i1 %tobool, ptr @t1, ptr @t2
+// CHECK: call void %cond(
+// CHECK: call void %cond2(
+// CHECK-NEXT:unreachable
+
+// CHECK-CXX-LABEL: @_Z16test_conditionali(
+// CHECK-CXX: %cond{{.*}} = phi ptr [ @_Z2t1i, %{{.*}} ], [ @_Z2t2i, %{{.*}} ]
+// CHECK-CXX: call void %cond{{.*}}(
+// CHECK-CXX: %cond{{.*}} = phi ptr [ @_Z2t1i, %{{.*}} ], [ @_Z2t1i, %{{.*}} ]
+// CHECK-CXX: call void %cond{{.*}}(
+// CHECK-CXX-NEXT:unreachable
+void t1(int) __attribute__((noreturn));
+void t2(int);
+__attribute__((noreturn)) void test_conditional(int a) {
+  // The conditional operator isn't noreturn because t2 isn't.
+  (a ? t1 : t2)(a);
+  // The conditional operator is noreturn.
+  (a ? t1 : t1)(a);
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -8262,7 +8262,9 @@
   lhptee = S.Context.getQualifiedType(lhptee.getUnqualifiedType(), lhQual);
   rhptee = S.Context.getQualifiedType(rhptee.getUnqualifiedType(), rhQual);
 
-  QualType CompositeTy = S.Context.mergeTypes(lhptee, rhptee);
+  QualType CompositeTy = S.Context.mergeTypes(
+  lhptee, rhptee, /*OfBlockPointer=*/false, /*Unqualified=*/false,
+  /*BlockReturnType=*/false, /*IsConditionalOperator=*/true);
 
   if (CompositeTy.isNull()) {
 // In this situation, we assume void* type. No especially good
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -10191,7 +10191,8 @@
 
 QualType ASTContext::mergeFunctionTypes(QualType lhs, QualType rhs,
 bool OfBlockPointer, bool Unqualified,
-bool AllowCXX) {
+bool AllowCXX,
+bool IsConditionalOperator) {
   const auto *lbase = lhs->castAs();
   const auto *rbase = rhs->castAs();
   const auto *lproto = dyn_cast(lbase);
@@ -10254,9 +10255,11 @@
   if (lbaseInfo.getNoCfCheck() != rbaseInfo.getNoCfCheck())
 return {};
 
-  // FIXME: some uses, e.g. conditional exprs, really want this to be 'both'.
-  bool NoReturn = lbaseInfo.getNoReturn() || rbaseInfo.getNoReturn();
-
+  // For ConditionalOperator, the result type is noreturn if both operands are
+  // noreturn.
+  bool NoReturn = IsConditionalOperator
+  ? lbaseInfo.getNoReturn() && rbaseInfo.getNoReturn()
+  : lbaseInfo.getNoReturn() || rbaseInfo.getNoReturn();
   if (lbaseInfo.getNoReturn() != NoReturn)
 allLTypes = false;
   if (rbaseInfo.getNoReturn() != NoReturn)
@@ -10389,9 +10392,9 @@
   return {};
 }
 
-QualType ASTContext::mergeTypes(QualType LHS, QualType RHS,
-bool OfBlockPointer,
-bool Unqualified, bool BlockReturnType) {
+QualType ASTContext::mergeTypes(QualType LHS, QualType RHS, bool OfBlockPointer,
+bool Unqualified, bool BlockReturnType,
+bool IsConditionalOperator) {
   // For C++ we will not reach this code with reference types (see below),
   // for OpenMP variant call overloading we might.
   //
@@ -10684,7 +10687,8 @@
   ArrayType::ArraySizeModifier(), 0);
   }
   case Type::FunctionNoProto:
-return mergeFunctionTypes(LHS,

[PATCH] D140868: [C] Make (c ? e1 : e2) noreturn only if both e1 and e2 are noreturn

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 485912.
MaskRay added a comment.

use `%itanium_abi_triple`. remove `-S`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140868

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CodeGen/attr-noreturn.c

Index: clang/test/CodeGen/attr-noreturn.c
===
--- clang/test/CodeGen/attr-noreturn.c
+++ clang/test/CodeGen/attr-noreturn.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -S -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -x c++ -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-CXX
 
 typedef void (*fptrs_t[4])(void);
 fptrs_t p __attribute__((noreturn));
@@ -8,3 +9,24 @@
 }
 // CHECK: call void
 // CHECK-NEXT: unreachable
+
+// CHECK-LABEL: @test_conditional(
+// CHECK: %cond = select i1 %tobool, ptr @t1, ptr @t2
+// CHECK: call void %cond(
+// CHECK: call void %cond2(
+// CHECK-NEXT:unreachable
+
+// CHECK-CXX-LABEL: @_Z16test_conditionali(
+// CHECK-CXX: %cond{{.*}} = phi ptr [ @_Z2t1i, %{{.*}} ], [ @_Z2t2i, %{{.*}} ]
+// CHECK-CXX: call void %cond{{.*}}(
+// CHECK-CXX: %cond{{.*}} = phi ptr [ @_Z2t1i, %{{.*}} ], [ @_Z2t1i, %{{.*}} ]
+// CHECK-CXX: call void %cond{{.*}}(
+// CHECK-CXX-NEXT:unreachable
+void t1(int) __attribute__((noreturn));
+void t2(int);
+__attribute__((noreturn)) void test_conditional(int a) {
+  // The conditional operator isn't noreturn because t2 isn't.
+  (a ? t1 : t2)(a);
+  // The conditional operator is noreturn.
+  (a ? t1 : t1)(a);
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -8262,7 +8262,9 @@
   lhptee = S.Context.getQualifiedType(lhptee.getUnqualifiedType(), lhQual);
   rhptee = S.Context.getQualifiedType(rhptee.getUnqualifiedType(), rhQual);
 
-  QualType CompositeTy = S.Context.mergeTypes(lhptee, rhptee);
+  QualType CompositeTy = S.Context.mergeTypes(
+  lhptee, rhptee, /*OfBlockPointer=*/false, /*Unqualified=*/false,
+  /*BlockReturnType=*/false, /*IsConditionalOperator=*/true);
 
   if (CompositeTy.isNull()) {
 // In this situation, we assume void* type. No especially good
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -10191,7 +10191,8 @@
 
 QualType ASTContext::mergeFunctionTypes(QualType lhs, QualType rhs,
 bool OfBlockPointer, bool Unqualified,
-bool AllowCXX) {
+bool AllowCXX,
+bool IsConditionalOperator) {
   const auto *lbase = lhs->castAs();
   const auto *rbase = rhs->castAs();
   const auto *lproto = dyn_cast(lbase);
@@ -10254,9 +10255,11 @@
   if (lbaseInfo.getNoCfCheck() != rbaseInfo.getNoCfCheck())
 return {};
 
-  // FIXME: some uses, e.g. conditional exprs, really want this to be 'both'.
-  bool NoReturn = lbaseInfo.getNoReturn() || rbaseInfo.getNoReturn();
-
+  // For ConditionalOperator, the result type is noreturn if both operands are
+  // noreturn.
+  bool NoReturn = IsConditionalOperator
+  ? lbaseInfo.getNoReturn() && rbaseInfo.getNoReturn()
+  : lbaseInfo.getNoReturn() || rbaseInfo.getNoReturn();
   if (lbaseInfo.getNoReturn() != NoReturn)
 allLTypes = false;
   if (rbaseInfo.getNoReturn() != NoReturn)
@@ -10389,9 +10392,9 @@
   return {};
 }
 
-QualType ASTContext::mergeTypes(QualType LHS, QualType RHS,
-bool OfBlockPointer,
-bool Unqualified, bool BlockReturnType) {
+QualType ASTContext::mergeTypes(QualType LHS, QualType RHS, bool OfBlockPointer,
+bool Unqualified, bool BlockReturnType,
+bool IsConditionalOperator) {
   // For C++ we will not reach this code with reference types (see below),
   // for OpenMP variant call overloading we might.
   //
@@ -10684,7 +10687,8 @@
   ArrayType::ArraySizeModifier(), 0);
   }
   case Type::FunctionNoProto:
-return mergeFunctionTypes(LHS, RHS, OfBlockPointer, Unqualified);
+return mergeFunctionTypes(LHS, RHS, OfBlockPointer, Unqualified,
+  /*AllowCXX=*/false, IsConditionalOperator);
   case Type::Record:
   case Type::Enum:
 return {};
Index: clang/include/clang/AST/ASTContext.h
===
--- clang/include/clang/AST/ASTContext.h
+++ clang/include/clang/AST/ASTContext.h
@@ -2865,10 +2865,12

[clang] d0ce367 - [C++20] [Modules] Fix a crash when instantiate hidden friends

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2023-01-03T13:37:57+08:00
New Revision: d0ce367a97137e6c2b28f1e18988fd7888bcea0a

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

LOG: [C++20] [Modules] Fix a crash when instantiate hidden friends

Closes https://github.com/llvm/llvm-project/issues/54457.

This removes a FIXME we found previously. But we didn't remove the FIXME
that time due to the lack of the corresponding test. And now we found
the corresponding test so we can remove it.

Added: 
clang/test/Modules/pr54457.cppm

Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a06f409d78d97..0fe0097400823 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -329,6 +329,8 @@ Bug Fixes
 - Fix sanity check when value initializing an empty union so that it takes into
   account anonymous structs which is a GNU extension. This fixes
   `Issue 58800 `_
+- Fix an issue that triggers a crash if we instantiate a hidden friend 
functions.
+  This fixes `Issue 54457 `_
 
 Improvements to Clang's diagnostics
 ^^^

diff  --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp 
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index a05eceac73988..a645ffe1cf291 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -6155,9 +6155,8 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc, 
NamedDecl *D,
 
   // Move to the outer template scope.
   if (FunctionDecl *FD = dyn_cast(DC)) {
-// FIXME: We should use `getNonTransparentDeclContext()` here instead
-// of `getDeclContext()` once we find the invalid test case.
-if (FD->getFriendObjectKind() && 
FD->getDeclContext()->isFileContext()){
+if (FD->getFriendObjectKind() &&
+FD->getNonTransparentDeclContext()->isFileContext()) {
   DC = FD->getLexicalDeclContext();
   continue;
 }

diff  --git a/clang/test/Modules/pr54457.cppm b/clang/test/Modules/pr54457.cppm
new file mode 100644
index 0..ed67ec1065376
--- /dev/null
+++ b/clang/test/Modules/pr54457.cppm
@@ -0,0 +1,61 @@
+// https://github.com/llvm/llvm-project/issues/54457
+//
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/A.cppm -verify -S -o -
+// RUN: %clang_cc1 -std=c++20 %t/B.cppm -verify -S -o -
+// RUN: %clang_cc1 -std=c++20 %t/C.cppm -emit-module-interface -o %t/C.pcm
+// RUN: %clang_cc1 -std=c++20 %t/UseC.cppm -fprebuilt-module-path=%t -verify 
-S -o -
+
+//--- A.cppm
+// expected-no-diagnostics
+export module A;
+
+export template
+struct s {
+   friend s f(s) {
+   return s();
+   }
+};
+
+void g() {
+   f(s());
+}
+
+//--- B.cppm
+// expected-no-diagnostics
+export module B;
+
+export template
+struct s {
+   friend constexpr auto f(s) -> s {
+   return s();
+   }
+};
+
+void g() {
+   constexpr auto first = f(s());
+}
+
+//--- C.cppm
+// expected-no-diagnostics
+export module C;
+
+export template
+struct basic_symbol_text {
+  template
+  constexpr friend basic_symbol_text operator+(
+const basic_symbol_text&, const basic_symbol_text&) noexcept
+  {
+return basic_symbol_text{};
+  }
+};
+
+constexpr auto xxx = basic_symbol_text{} + basic_symbol_text{};
+
+//--- UseC.cppm
+// expected-no-diagnostics
+import C;
+void foo() {}



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

[PATCH] D140817: [Driver] move NetBSD header search path management to the driver

[Fangrui Song via Phabricator via cfe-commits]

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

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140817

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

[PATCH] D140685: [LoongArch] Add intrinsics for MOVFCSR2GR and MOVGR2FCSR instructions

[Xiaodong Liu via Phabricator via cfe-commits]

XiaodongLoong updated this revision to Diff 485921.
XiaodongLoong added a comment.

rebase code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140685

Files:
  clang/include/clang/Basic/BuiltinsLoongArch.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/larchintrin.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/LoongArch/intrinsic-la32-error.c
  clang/test/CodeGen/LoongArch/intrinsic-la32.c
  clang/test/CodeGen/LoongArch/intrinsic-la64-error.c
  clang/test/CodeGen/LoongArch/intrinsic-la64.c
  llvm/include/llvm/IR/IntrinsicsLoongArch.td
  llvm/lib/Target/LoongArch/LoongArchFloat32InstrInfo.td
  llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
  llvm/lib/Target/LoongArch/LoongArchISelLowering.h
  llvm/lib/Target/LoongArch/LoongArchInstrInfo.td
  llvm/test/CodeGen/LoongArch/intrinsic-error.ll
  llvm/test/CodeGen/LoongArch/intrinsic-not-constant-error.ll
  llvm/test/CodeGen/LoongArch/intrinsic.ll

Index: llvm/test/CodeGen/LoongArch/intrinsic.ll
===
--- llvm/test/CodeGen/LoongArch/intrinsic.ll
+++ llvm/test/CodeGen/LoongArch/intrinsic.ll
@@ -1,10 +1,12 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc --mtriple=loongarch32 < %s | FileCheck %s
-; RUN: llc --mtriple=loongarch64 < %s | FileCheck %s
+; RUN: llc --mtriple=loongarch32 --mattr=+f < %s | FileCheck %s
+; RUN: llc --mtriple=loongarch64 --mattr=+f < %s | FileCheck %s
 
 declare void @llvm.loongarch.dbar(i32)
 declare void @llvm.loongarch.ibar(i32)
 declare void @llvm.loongarch.break(i32)
+declare void @llvm.loongarch.movgr2fcsr(i32, i32)
+declare i32 @llvm.loongarch.movfcsr2gr(i32)
 declare void @llvm.loongarch.syscall(i32)
 declare i32 @llvm.loongarch.csrrd.w(i32 immarg)
 declare i32 @llvm.loongarch.csrwr.w(i32, i32 immarg)
@@ -47,6 +49,26 @@
   ret void
 }
 
+define void @movgr2fcsr(i32 %a) nounwind {
+; CHECK-LABEL: movgr2fcsr:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:movgr2fcsr $fcsr1, $a0
+; CHECK-NEXT:ret
+entry:
+  call void @llvm.loongarch.movgr2fcsr(i32 1, i32 %a)
+  ret void
+}
+
+define i32 @movfcsr2gr() nounwind {
+; CHECK-LABEL: movfcsr2gr:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:movfcsr2gr $a0, $fcsr1
+; CHECK-NEXT:ret
+entry:
+  %res = call i32 @llvm.loongarch.movfcsr2gr(i32 1)
+  ret i32 %res
+}
+
 define void @syscall() nounwind {
 ; CHECK-LABEL: syscall:
 ; CHECK:   # %bb.0: # %entry
Index: llvm/test/CodeGen/LoongArch/intrinsic-not-constant-error.ll
===
--- llvm/test/CodeGen/LoongArch/intrinsic-not-constant-error.ll
+++ llvm/test/CodeGen/LoongArch/intrinsic-not-constant-error.ll
@@ -4,6 +4,8 @@
 declare void @llvm.loongarch.dbar(i32)
 declare void @llvm.loongarch.ibar(i32)
 declare void @llvm.loongarch.break(i32)
+declare void @llvm.loongarch.movgr2fcsr(i32, i32)
+declare i32 @llvm.loongarch.movfcsr2gr(i32)
 declare void @llvm.loongarch.syscall(i32)
 
 define void @dbar_not_constant(i32 %x) nounwind {
@@ -27,6 +29,20 @@
   ret void
 }
 
+define void @movgr2fcsr(i32 %a) nounwind {
+; CHECK: immarg operand has non-immediate parameter
+entry:
+  call void @llvm.loongarch.movgr2fcsr(i32 %a, i32 %a)
+  ret void
+}
+
+define i32 @movfcsr2gr(i32 %a) nounwind {
+; CHECK: immarg operand has non-immediate parameter
+entry:
+  %res = call i32 @llvm.loongarch.movfcsr2gr(i32 %a)
+  ret i32 %res
+}
+
 define void @syscall(i32 %x) nounwind {
 ; CHECK: immarg operand has non-immediate parameter
 entry:
Index: llvm/test/CodeGen/LoongArch/intrinsic-error.ll
===
--- llvm/test/CodeGen/LoongArch/intrinsic-error.ll
+++ llvm/test/CodeGen/LoongArch/intrinsic-error.ll
@@ -5,105 +5,151 @@
 declare void @llvm.loongarch.dbar(i32)
 declare void @llvm.loongarch.ibar(i32)
 declare void @llvm.loongarch.break(i32)
+declare void @llvm.loongarch.movgr2fcsr(i32, i32)
+declare i32 @llvm.loongarch.movfcsr2gr(i32)
 declare void @llvm.loongarch.syscall(i32)
 declare i32 @llvm.loongarch.csrrd.w(i32 immarg)
 declare i32 @llvm.loongarch.csrwr.w(i32, i32 immarg)
 declare i32 @llvm.loongarch.csrxchg.w(i32, i32, i32 immarg)
 
-define void @dbar_imm_out_of_hi_range() nounwind {
+define void @dbar_imm_out_of_hi_range() #0 {
 ; CHECK: argument to 'llvm.loongarch.dbar' out of range
 entry:
   call void @llvm.loongarch.dbar(i32 32769)
   ret void
 }
 
-define void @dbar_imm_out_of_lo_range() nounwind {
+define void @dbar_imm_out_of_lo_range() #0 {
 ; CHECK: argument to 'llvm.loongarch.dbar' out of range
 entry:
   call void @llvm.loongarch.dbar(i32 -1)
   ret void
 }
 
-define void @ibar_imm_out_of_hi_range() nounwind {
+define void @ibar_imm_out_of_hi_range() #0 {
 ; CHECK: argument to 'llvm.loongarch.ibar' out of range
 entry:
   call void @llvm.loongarch.ibar(i32 32769)
   ret void
 }
 
-define void @ibar_imm_ou

[PATCH] D140775: [clangd] Hover: show CalleeArgInfo for literals

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

In D140775#4021967 , @tom-anders 
wrote:

> In D140775#4021634 , @nridge wrote:
>
>> Do you want to add a simple test case for a non-literal expression? 
>> Something like hovering over the `+` in `2 + 3` should work.
>
> Will do!
>
>> Also, this is pre-existing, but I noticed that in a case like this:
>>
>>   void bar(int);
>>   void func() {
>> int x = 42;
>> bar(x);
>>   }
>>
>> the hover for `x` includes a line that just says `Passed` -- is that useful 
>> in any way? Should we just omit the CalleeArgInfo a case like that?
>
> Hmm, the thing is that if the function signature is `void bar(int&)`, then 
> "Passed by reference" would be useful again, so we can't just add a `if 
> (!CalleeArgInfo->Name.empty())` around the `OS << "Passed ";` Maybe we could 
> instead display "Passed **by value**" if and only if the name is empty?

That sounds fine to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140775

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

[PATCH] D140814: [clangd] show underlying type in type hint for `decltype(expr)`

[Nathan Ridge via Phabricator via cfe-commits]

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

Thanks!




Comment at: clang-tools-extra/clangd/InlayHints.cpp:665
 
+  // The sugared type is more useful in some cases, and the canonical
+  // type in other cases. For now, prefer the sugared type unless

small nit: could you put the comment inside the function, since it's more an 
explanation of the current logic in the implementation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140814

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

[clang] 367e618 - [C++20] [Modules] Emit full specialization of variable template as available externally in importers

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2023-01-03T14:48:29+08:00
New Revision: 367e618fd605353ad77bb78f884c758948f0d573

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

LOG: [C++20] [Modules] Emit full specialization of variable template as 
available externally in importers

Closes https://github.com/llvm/llvm-project/issues/59780.

In this issue report, when we use full specialization of variable
templates in modules, we will meet the multiple definition errors.

The root cause of the problem is that when we see the full
specialization of the variable template in the importers, we will find
if it is already defined in the external sources and we failed to find
such definitions from external sources. So we generate the definition in
the current TU. We failed to find the definition in the external sources
because we restricted to not write it during writing. However, we don't
know why we restricted it and it doesn't make a lot sense to do such
restriction. Then no test fails after we remove such limitations. So
let's remove it now and add it back later if we found it is necessary
then we can add the corresponding test that time.

Note that the code is only applied to named modules and
PCHWithObjectFiles. So it won't affect the normal clang modules and
header units.

Added: 
clang/test/Modules/pr59780.cppm

Modified: 
clang/lib/Serialization/ASTWriterDecl.cpp

Removed: 




diff  --git a/clang/lib/Serialization/ASTWriterDecl.cpp 
b/clang/lib/Serialization/ASTWriterDecl.cpp
index 353a58507cb2..c117a29723a7 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1037,8 +1037,7 @@ void ASTDeclWriter::VisitVarDecl(VarDecl *D) {
   if (D->getStorageDuration() == SD_Static) {
 bool ModulesCodegen = false;
 if (Writer.WritingModule &&
-!D->getDescribedVarTemplate() && !D->getMemberSpecializationInfo() &&
-!isa(D)) {
+!D->getDescribedVarTemplate() && !D->getMemberSpecializationInfo()) {
   // When building a C++20 module interface unit or a partition unit, a
   // strong definition in the module interface is provided by the
   // compilation of that unit, not by its users. (Inline variables are 
still

diff  --git a/clang/test/Modules/pr59780.cppm b/clang/test/Modules/pr59780.cppm
new file mode 100644
index ..9578325b976b
--- /dev/null
+++ b/clang/test/Modules/pr59780.cppm
@@ -0,0 +1,46 @@
+// https://github.com/llvm/llvm-project/issues/59780
+//
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/a.cppm -triple %itanium_abi_triple 
-emit-module-interface -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 %t/use.cpp -fprebuilt-module-path=%t -S \
+// RUN: -triple %itanium_abi_triple -emit-llvm -o - | FileCheck %t/use.cpp
+// RUN: %clang_cc1 -std=c++20 %t/a.pcm -triple %itanium_abi_triple -emit-llvm 
-o - | FileCheck %t/a.cppm
+
+//--- a.cppm
+export module a;
+
+export template
+int x = 0;
+
+export template<>
+int x = 0;
+
+export template
+struct Y {
+static int value;
+};
+
+template 
+int Y::value = 0;
+
+export template<>
+struct Y {
+static int value;
+};
+
+int Y::value = 0;
+
+// CHECK-NOT: @_ZW1a1xIiE = {{.*}}available_externally{{.*}}global
+// CHECK-NOT: @_ZNW1a1YIiE5valueE = {{.*}}available_externally{{.*}}global
+
+//--- use.cpp
+import a;
+int foo() {
+return x + Y::value;
+}
+
+// CHECK: @_ZW1a1xIiE = {{.*}}available_externally{{.*}}global
+// CHECK: @_ZNW1a1YIiE5valueE = {{.*}}available_externally{{.*}}global



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

[PATCH] D140814: [clangd] show underlying type in type hint for `decltype(expr)`

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 485923.
v1nh1shungry added a comment.

move the comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140814

Files:
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1364,7 +1364,6 @@
 TEST(TypeHints, Decltype) {
   assertTypeHints(R"cpp(
 $a[[decltype(0)]] a;
-// FIXME: will be nice to show `: int` instead
 $b[[decltype(a)]] b;
 const $c[[decltype(0)]] &c = b;
 
@@ -1377,11 +1376,13 @@
 
 template  struct Foo;
 using G = Foo<$g[[decltype(0)]], float>;
+
+auto $h[[h]] = $i[[decltype(0)]]{};
   )cpp",
-  ExpectedHint{": int", "a"},
-  ExpectedHint{": decltype(0)", "b"},
+  ExpectedHint{": int", "a"}, ExpectedHint{": int", "b"},
   ExpectedHint{": int", "c"}, ExpectedHint{": int", "e"},
-  ExpectedHint{": int", "f"}, ExpectedHint{": int", "g"});
+  ExpectedHint{": int", "f"}, ExpectedHint{": int", "g"},
+  ExpectedHint{": int", "h"}, ExpectedHint{": int", "i"});
 }
 
 TEST(DesignatorHints, Basic) {
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -662,7 +662,21 @@
  sourceLocToPosition(SM, Spelled->back().endLocation())};
   }
 
+  static bool shouldPrintCanonicalType(QualType QT) {
+// The sugared type is more useful in some cases, and the canonical
+// type in other cases. For now, prefer the sugared type unless
+// we are printing `decltype(expr)`. This could be refined further
+// (see https://github.com/clangd/clangd/issues/1298).
+if (QT->isDecltypeType())
+  return true;
+if (const AutoType *AT = QT->getContainedAutoType())
+  if (AT->getDeducedType()->isDecltypeType())
+return true;
+return false;
+  }
+
   void addTypeHint(SourceRange R, QualType T, llvm::StringRef Prefix) {
+TypeHintPolicy.PrintCanonicalTypes = shouldPrintCanonicalType(T);
 addTypeHint(R, T, Prefix, TypeHintPolicy);
   }
 


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1364,7 +1364,6 @@
 TEST(TypeHints, Decltype) {
   assertTypeHints(R"cpp(
 $a[[decltype(0)]] a;
-// FIXME: will be nice to show `: int` instead
 $b[[decltype(a)]] b;
 const $c[[decltype(0)]] &c = b;
 
@@ -1377,11 +1376,13 @@
 
 template  struct Foo;
 using G = Foo<$g[[decltype(0)]], float>;
+
+auto $h[[h]] = $i[[decltype(0)]]{};
   )cpp",
-  ExpectedHint{": int", "a"},
-  ExpectedHint{": decltype(0)", "b"},
+  ExpectedHint{": int", "a"}, ExpectedHint{": int", "b"},
   ExpectedHint{": int", "c"}, ExpectedHint{": int", "e"},
-  ExpectedHint{": int", "f"}, ExpectedHint{": int", "g"});
+  ExpectedHint{": int", "f"}, ExpectedHint{": int", "g"},
+  ExpectedHint{": int", "h"}, ExpectedHint{": int", "i"});
 }
 
 TEST(DesignatorHints, Basic) {
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -662,7 +662,21 @@
  sourceLocToPosition(SM, Spelled->back().endLocation())};
   }
 
+  static bool shouldPrintCanonicalType(QualType QT) {
+// The sugared type is more useful in some cases, and the canonical
+// type in other cases. For now, prefer the sugared type unless
+// we are printing `decltype(expr)`. This could be refined further
+// (see https://github.com/clangd/clangd/issues/1298).
+if (QT->isDecltypeType())
+  return true;
+if (const AutoType *AT = QT->getContainedAutoType())
+  if (AT->getDeducedType()->isDecltypeType())
+return true;
+return false;
+  }
+
   void addTypeHint(SourceRange R, QualType T, llvm::StringRef Prefix) {
+TypeHintPolicy.PrintCanonicalTypes = shouldPrintCanonicalType(T);
 addTypeHint(R, T, Prefix, TypeHintPolicy);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits