[PATCH] D112359: [RISCV] Unify depedency check and extension implication parsing logics

2021-11-28 Thread Yueh-Ting Chen via Phabricator via cfe-commits
eopXD updated this revision to Diff 390187.
eopXD added a comment.

- Update code (zfh have been added recently.
- Added test case `attribute-arch-invalid.s` to show that experimental 
extensions need to specify the version explicitly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112359

Files:
  clang/test/CodeGen/RISCV/riscv-metadata.c
  clang/test/CodeGen/RISCV/riscv32-ilp32-ilp32f-ilp32d-abi.c
  clang/test/CodeGen/RISCV/riscv32-ilp32d-abi.c
  clang/test/CodeGen/RISCV/riscv32-ilp32f-ilp32d-abi.c
  clang/test/CodeGen/RISCV/riscv64-lp64-lp64f-lp64d-abi.c
  clang/test/CodeGen/RISCV/riscv64-lp64d-abi.c
  clang/test/CodeGen/RISCV/riscv64-lp64f-lp64d-abi.c
  clang/test/CodeGen/riscv32-ilp32d-abi.cpp
  llvm/include/llvm/Support/RISCVISAInfo.h
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/test/MC/RISCV/attribute-arch-invalid.s
  llvm/test/MC/RISCV/attribute-arch.s

Index: llvm/test/MC/RISCV/attribute-arch.s
===
--- llvm/test/MC/RISCV/attribute-arch.s
+++ llvm/test/MC/RISCV/attribute-arch.s
@@ -33,47 +33,46 @@
 .attribute arch, "rv32ima2p0_fdc"
 # CHECK: attribute  5, "rv32i2p0_m2p0_a2p0_f2p0_d2p0_c2p0"
 
-.attribute arch, "rv32iv"
+## Experimental extensions require version string to be explicitly specified
+
+.attribute arch, "rv32iv0p10"
 # CHECK: attribute  5, "rv32i2p0_v0p10_zvlsseg0p10"
 
-.attribute arch, "rv32izba"
+.attribute arch, "rv32izba1p0"
 # CHECK: attribute  5, "rv32i2p0_zba1p0"
 
-.attribute arch, "rv32izbb"
+.attribute arch, "rv32izbb1p0"
 # CHECK: attribute  5, "rv32i2p0_zbb1p0"
 
-.attribute arch, "rv32izbc"
+.attribute arch, "rv32izbc1p0"
 # CHECK: attribute  5, "rv32i2p0_zbc1p0"
 
-.attribute arch, "rv32izbe"
+.attribute arch, "rv32izbe0p93"
 # CHECK: attribute  5, "rv32i2p0_zbe0p93"
 
-.attribute arch, "rv32izbf"
+.attribute arch, "rv32izbf0p93"
 # CHECK: attribute  5, "rv32i2p0_zbf0p93"
 
-.attribute arch, "rv32izbm"
+.attribute arch, "rv32izbm0p93"
 # CHECK: attribute  5, "rv32i2p0_zbm0p93"
 
-.attribute arch, "rv32izbp"
+.attribute arch, "rv32izbp0p93"
 # CHECK: attribute  5, "rv32i2p0_zbp0p93"
 
-.attribute arch, "rv32izbr"
+.attribute arch, "rv32izbr0p93"
 # CHECK: attribute  5, "rv32i2p0_zbr0p93"
 
-.attribute arch, "rv32izbs"
+.attribute arch, "rv32izbs1p0"
 # CHECK: attribute  5, "rv32i2p0_zbs1p0"
 
-.attribute arch, "rv32izbt"
+.attribute arch, "rv32izbt0p93"
 # CHECK: attribute  5, "rv32i2p0_zbt0p93"
 
-.attribute arch, "rv32ifzfhmin"
+.attribute arch, "rv32ifzfhmin0p1"
 # CHECK: attribute  5, "rv32i2p0_f2p0_zfhmin0p1"
 
-.attribute arch, "rv32ifzfh"
+.attribute arch, "rv32ifzfh0p1"
 # CHECK: attribute  5, "rv32i2p0_f2p0_zfh0p1_zfhmin0p1"
 
-.attribute arch, "rv32ivzvamo_zvlsseg"
-# CHECK: attribute  5, "rv32i2p0_v0p10_zvamo0p10_zvlsseg0p10"
-
-.attribute arch, "rv32iv_zvamo0p10_zvlsseg"
+.attribute arch, "rv32iv0p10zvamo0p10_zvlsseg0p10"
 # CHECK: attribute  5, "rv32i2p0_v0p10_zvamo0p10_zvlsseg0p10"
Index: llvm/test/MC/RISCV/attribute-arch-invalid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/attribute-arch-invalid.s
@@ -0,0 +1,48 @@
+## Invalid arch string
+
+# RUN: not llvm-mc -triple riscv32 < %s 2>&1 | FileCheck %s
+# RUN: not llvm-mc -triple riscv64 < %s 2>&1 | FileCheck %s
+
+## Version strings are required for experimental extensions
+
+.attribute arch, "rv32iv"
+# CHECK: error: invalid arch name 'rv32iv', experimental extension requires explicit version number `v`
+
+.attribute arch, "rv32izba"
+# CHECK: error: invalid arch name 'rv32izba', experimental extension requires explicit version number `zba`
+
+.attribute arch, "rv32izbb"
+# CHECK: error: invalid arch name 'rv32izbb', experimental extension requires explicit version number `zbb`
+
+.attribute arch, "rv32izbc"
+# CHECK: error: invalid arch name 'rv32izbc', experimental extension requires explicit version number `zbc`
+
+.attribute arch, "rv32izbe"
+# CHECK:  error: invalid arch name 'rv32izbe', experimental extension requires explicit version number `zbe`
+
+.attribute arch, "rv32izbf"
+# CHECK: error: invalid arch name 'rv32izbf', experimental extension requires explicit version number `zbf`
+
+.attribute arch, "rv32izbm"
+# CHECK: error: invalid arch name 'rv32izbm', experimental extension requires explicit version number `zbm`
+
+.attribute arch, "rv32izbp"
+# CHECK: error: invalid arch name 'rv32izbp', experimental extension requires explicit version number `zbp`
+
+.attribute arch, "rv32izbr"
+# CHECK: error: invalid arch name 'rv32izbr', experimental extension requires explicit version number `zbr`
+
+.attribute arch, "rv32izbs"
+# CHECK: error: invalid arch name 'rv32izbs', experimental extension requires explicit version number `zbs`
+
+.attribute arch, "rv32izbt"
+# CHECK: error: 

[PATCH] D114427: [clang-tidy] Warn on functional C-style casts

2021-11-28 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 390191.
carlosgalvezp added a comment.

Add extra test for template functions that use the functional cast. It should 
only warn when T is not a class type.


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

https://reviews.llvm.org/D114427

Files:
  clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp
@@ -143,11 +143,10 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: redundant cast to the same type
   // CHECK-FIXES: {{^}}  kZero;
 
-  int b2 = int(b);
-  int b3 = static_cast(b);
-  int b4 = b;
+  int b2 = static_cast(b);
+  int b3 = b;
   double aa = a;
-  (void)b2;
+  (void)aa;
   return (void)g();
 }
 
@@ -321,3 +320,43 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: C-style casts are discouraged; use constructor call syntax [
   // CHECK-FIXES: auto s6 = S(cr);
 }
+
+template 
+T functional_cast_template_used_by_class(float i) {
+  return T(i);
+}
+
+template 
+T functional_cast_template_used_by_int(float i) {
+  return T(i);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: C-style casts are discouraged; use static_cast
+  // CHECK-FIXES: return static_cast(i);
+}
+
+struct S2 {
+  S2(float) {}
+};
+
+void functional_casts() {
+  float x = 1.5F;
+  auto y = int(x);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: C-style casts are discouraged; use static_cast
+  // CHECK-FIXES: auto y = static_cast(x);
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wc++11-narrowing"
+  // This if fine, compiler will warn about implicit conversions with brace initialization
+  auto z = int{x};
+#pragma clang diagnostic pop
+
+  // Functional casts are allowed if S is of class type
+  const char *str = "foo";
+  auto s = S(str);
+
+  // Functional casts in template functions
+  functional_cast_template_used_by_class(x);
+  functional_cast_template_used_by_int(x);
+
+  // New expressions are not functional casts
+  auto w = new int(x);
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -133,9 +133,11 @@
 Changes in existing checks
 ^^
 
-- Removed default setting `cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"`,
+- Removed default setting ``cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"``,
   to match the current state of the C++ Core Guidelines.
 
+- Updated ``google-readability-casting`` to diagnose and fix functional casts, to achieve feature
+  parity with the corresponding ``cpplint.py`` check.
 
 Removed checks
 ^^
Index: clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
+++ clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
@@ -31,6 +31,11 @@
   unless(isInTemplateInstantiation()))
   .bind("cast"),
   this);
+  Finder->addMatcher(
+  cxxFunctionalCastExpr(unless(hasDescendant(cxxConstructExpr())),
+unless(hasDescendant(initListExpr(
+  .bind("cast"),
+  this);
 }
 
 static bool needsConstCast(QualType SourceType, QualType DestType) {
@@ -55,8 +60,39 @@
   return T1.getUnqualifiedType() == T2.getUnqualifiedType();
 }
 
+static clang::CharSourceRange getReplaceRange(const CStyleCastExpr *CastExpr) {
+  return CharSourceRange::getCharRange(
+  CastExpr->getLParenLoc(), CastExpr->getSubExprAsWritten()->getBeginLoc());
+}
+
+static clang::CharSourceRange
+getReplaceRange(const CXXFunctionalCastExpr *CastExpr) {
+  return CharSourceRange::getCharRange(CastExpr->getBeginLoc(),
+   CastExpr->getLParenLoc());
+}
+
+static StringRef getDestTypeString(const SourceManager &SM,
+   const LangOptions &LangOpts,
+   const CStyleCastExpr *CastExpr) {
+  return Lexer::getSourceText(
+  CharSourceRange::getTokenRange(
+  CastExpr->getLParenLoc().getLocWithOffset(1),
+  CastExpr->getRParenLoc().getLocWithOffset(-1)),
+  SM, LangOpts);
+}
+
+static StringRef getDestTypeString(const SourceManager &SM,
+   const LangOptions &LangOpts,
+   const CXXFunctionalCastExpr *CastExpr) {
+  return Lexer::getSourceText(
+  CharSourceRange::getTokenRange(
+  CastExpr->getBegi

[PATCH] D114427: [clang-tidy] Warn on functional C-style casts

2021-11-28 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp:342
+  auto w = new int(x);
+}

Quuxplusone wrote:
> What about
> ```
> template T foo(int i) { return T(i); }
> int main() {
> foo>(); // valid, OK(!)
> foo(); // valid, not OK
> }
> ```
> What about
> ```
> struct Widget { Widget(int); };
> using T = Widget;
> using U = Widget&;
> int i = 42;
> Widget t = T(i);  // valid, OK?
> Widget u = U(i);  // valid C++, should definitely not be OK
> ```
> https://quuxplusone.github.io/blog/2020/01/22/expression-list-in-functional-cast/
Good point, thanks! I've added the first one to the unit test.

Regarding the second check, I'm not sure if it's the scope of this check. This 
check does not care whether the constructor of the class is implicit or not - 
if you use a class type, then you are calling the constructor so it's fine. 
Same goes when it's a reference - in my opinion this check is not concerned 
with that.

I definitely see the problems that can arise from the example that you posted, 
but maybe it fits better as a separate check in the `bugprone` category? This 
check (`google-readability-casting`) is focused only about avoiding C-style 
casting, i.e. it's a readability/style/modernize matter IMO. If cpplint is not 
diagnosing that, I don't think this check should either.


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

https://reviews.llvm.org/D114427

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


[PATCH] D114649: [libc++] Implement not-yet-voted LWG3436

2021-11-28 Thread Mark de Wever via Phabricator via cfe-commits
Mordante added a comment.

Since this patch requires both an update to both Clang and libc++ I think it 
would land the Clang part in a separate patch. Then wait for the CI to have ToT 
with this version and land that separately.
For now can you run the CI with the Bootstrap build?




Comment at: libcxx/include/__memory/construct_at.h:40
+// This implements https://wg21.link/LWG3436, which hasn't been voted into 
the Standard yet.
+if constexpr (is_array_v<_Tp>)
+return _VSTD::launder(__location);

Please add here also you implement the resolution of 2021-01-16.
I was wondering whether we should add it to the status list for C++23, but 
looking at the issue it's not certain it will be in C++23.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114649

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


[PATCH] D111617: [RISCV] Lazily add RVV C intrinsics.

2021-11-28 Thread Hsiangkai Wang via Phabricator via cfe-commits
HsiangKai updated this revision to Diff 390198.
HsiangKai added a comment.

Address a part of comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111617

Files:
  clang/include/clang/Basic/CMakeLists.txt
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/utils/TableGen/RISCVVEmitter.cpp
  clang/utils/TableGen/TableGen.cpp
  clang/utils/TableGen/TableGenBackends.h
  llvm/docs/CommandGuide/tblgen.rst

Index: llvm/docs/CommandGuide/tblgen.rst
===
--- llvm/docs/CommandGuide/tblgen.rst
+++ llvm/docs/CommandGuide/tblgen.rst
@@ -348,6 +348,10 @@
 
   Generate ``riscv_vector_builtin_cg.inc`` for Clang.
 
+.. option:: -gen-riscv-vector-builtin-sema
+
+  Generate ``riscv_vector_builtin_sema.inc`` for Clang.
+
 .. option:: -gen-attr-docs
 
   Generate attribute documentation.
Index: clang/utils/TableGen/TableGenBackends.h
===
--- clang/utils/TableGen/TableGenBackends.h
+++ clang/utils/TableGen/TableGenBackends.h
@@ -110,6 +110,7 @@
 void EmitRVVHeader(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 void EmitRVVBuiltins(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 void EmitRVVBuiltinCG(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
+void EmitRVVBuiltinSema(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 
 void EmitCdeHeader(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 void EmitCdeBuiltinDef(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
Index: clang/utils/TableGen/TableGen.cpp
===
--- clang/utils/TableGen/TableGen.cpp
+++ clang/utils/TableGen/TableGen.cpp
@@ -88,6 +88,7 @@
   GenRISCVVectorHeader,
   GenRISCVVectorBuiltins,
   GenRISCVVectorBuiltinCG,
+  GenRISCVVectorBuiltinSema,
   GenAttrDocs,
   GenDiagDocs,
   GenOptDocs,
@@ -243,6 +244,8 @@
"Generate riscv_vector_builtins.inc for clang"),
 clEnumValN(GenRISCVVectorBuiltinCG, "gen-riscv-vector-builtin-codegen",
"Generate riscv_vector_builtin_cg.inc for clang"),
+clEnumValN(GenRISCVVectorBuiltinSema, "gen-riscv-vector-builtin-sema",
+   "Generate riscv_vector_builtin_sema.inc for clang"),
 clEnumValN(GenAttrDocs, "gen-attr-docs",
"Generate attribute documentation"),
 clEnumValN(GenDiagDocs, "gen-diag-docs",
@@ -458,6 +461,9 @@
   case GenRISCVVectorBuiltinCG:
 EmitRVVBuiltinCG(Records, OS);
 break;
+  case GenRISCVVectorBuiltinSema:
+EmitRVVBuiltinSema(Records, OS);
+break;
   case GenAttrDocs:
 EmitClangAttrDocs(Records, OS);
 break;
Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -15,6 +15,7 @@
 //===--===//
 
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringMap.h"
@@ -22,6 +23,8 @@
 #include "llvm/ADT/Twine.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
+#include "llvm/TableGen/StringMatcher.h"
+#include "llvm/TableGen/TableGenBackend.h"
 #include 
 
 using namespace llvm;
@@ -55,7 +58,6 @@
 Float,
 Invalid,
   };
-  BasicType BT;
   ScalarTypeKind ScalarType = Invalid;
   LMULType LMUL;
   bool IsPointer = false;
@@ -71,11 +73,30 @@
   std::string ClangBuiltinStr;
   std::string Str;
   std::string ShortStr;
+  std::string QualExpr;
+  std::string MangledStr;
 
 public:
   RVVType() : RVVType(BasicType(), 0, StringRef()) {}
   RVVType(BasicType BT, int Log2LMUL, StringRef prototype);
 
+  StringRef getMangledStr() {
+if (MangledStr.empty()) {
+  if (!Valid)
+MangledStr = "invalid";
+  else {
+MangledStr =
+(Twine(ScalarType) + Twine(IsPointer) + Twine(IsImmediate) +
+ Twine(IsConstant) + Twine(ElementBitwidth))
+.str();
+if (!isScalar())
+  MangledStr += (Twine(Scale.getValue()) + LMUL.str()).str();
+  }
+}
+
+return MangledStr;
+  }
+
   // Return the string representation of a type, which is an encoded string for
   // passing to the BUILTIN() macro in Builtins.def.
   const std::string &getBuiltinStr() const { return BuiltinStr; }
@@ -97,6 +118,8 @@
 return ShortStr;
   }
 
+  const std::string &getQualExpr() { return QualExpr; }
+
   bool isValid() const { return Valid; }
   bool isScalar() const { return Scale.hasValue() && Scale.getValue() == 0; }
   bool isVector() const { return Scale.hasValue() && Scale.getValue() != 0; }
@@ -110,13 +133,15 @@

[PATCH] D114427: [clang-tidy] Warn on functional C-style casts

2021-11-28 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp:342
+  auto w = new int(x);
+}

carlosgalvezp wrote:
> Quuxplusone wrote:
> > What about
> > ```
> > template T foo(int i) { return T(i); }
> > int main() {
> > foo>(); // valid, OK(!)
> > foo(); // valid, not OK
> > }
> > ```
> > What about
> > ```
> > struct Widget { Widget(int); };
> > using T = Widget;
> > using U = Widget&;
> > int i = 42;
> > Widget t = T(i);  // valid, OK?
> > Widget u = U(i);  // valid C++, should definitely not be OK
> > ```
> > https://quuxplusone.github.io/blog/2020/01/22/expression-list-in-functional-cast/
> Good point, thanks! I've added the first one to the unit test.
> 
> Regarding the second check, I'm not sure if it's the scope of this check. 
> This check does not care whether the constructor of the class is implicit or 
> not - if you use a class type, then you are calling the constructor so it's 
> fine. Same goes when it's a reference - in my opinion this check is not 
> concerned with that.
> 
> I definitely see the problems that can arise from the example that you 
> posted, but maybe it fits better as a separate check in the `bugprone` 
> category? This check (`google-readability-casting`) is focused only about 
> avoiding C-style casting, i.e. it's a readability/style/modernize matter IMO. 
> If cpplint is not diagnosing that, I don't think this check should either.
It seems I should be able to just add the second example as a test and 
clang-tidy would warn but, what should be the fixit for it? A `static_cast` 
would lead to compiler error (which I personally would gladly take, but I don't 
know in general if we want clang-tidy to "fix" code leading to compiler 
errors"). Adding an ad-hoc message for this particular error seems out of the 
scope of a "readability" check. 

What do you guys think?


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

https://reviews.llvm.org/D114427

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


[PATCH] D114427: [clang-tidy] Warn on functional C-style casts

2021-11-28 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment.

Personally I think it's best to merge this as is, allowing people to e.g. 
replace cpplint with clang-tidy (one static analyzer to rule them all!), and 
add more functionality in a separate, focused patch. Looking forward to hearing 
your thoughts :)


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

https://reviews.llvm.org/D114427

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


[PATCH] D114660: [clang] Fix -Wreturn-type false positive in @try statements

2021-11-28 Thread Hans Wennborg via Phabricator via cfe-commits
hans accepted this revision.
hans added a comment.
This revision is now accepted and ready to land.

lgtm


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

https://reviews.llvm.org/D114660

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


[PATCH] D114660: [clang] Fix -Wreturn-type false positive in @try statements

2021-11-28 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

For future self, related changes I looked at:

https://reviews.llvm.org/rG04c6851cd6053 (made it so that calls don't add edges 
to try cfg blocks)
https://reviews.llvm.org/rG33979f75a0fd8 (added -Wreturn-type in the first 
place)
https://reviews.llvm.org/rG0c2ec779cf680d22831de618b32c6aad626c0b89 (moved all 
this code around)
https://reviews.llvm.org/rG918fe8498dfd7 (moved all this code around again, to 
its current location)


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

https://reviews.llvm.org/D114660

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


[PATCH] D114660: [clang] Fix -Wreturn-type false positive in @try statements

2021-11-28 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

https://reviews.llvm.org/rG06b602e540cd94d09ccbb39d63b151860a4022d2 (added the 
c++ exception checks to the warning)


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

https://reviews.llvm.org/D114660

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


[clang] 8874ada - [clang] Fix -Wreturn-type false positive in @try statements

2021-11-28 Thread Nico Weber via cfe-commits

Author: Nico Weber
Date: 2021-11-28T12:56:46-05:00
New Revision: 8874ada906f6a2ae0c4aa978581a0c9c26b89ffd

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

LOG: [clang] Fix -Wreturn-type false positive in @try statements

After 04f30795f16638, -Wreturn-type has an effect on functions that
contain @try/@catch statements. CheckFallThrough() was missing
a case for ObjCAtTryStmts, leading to a false positive.

(What about the other two places in CheckFallThrough() that handle
CXXTryStmt but not ObjCAtTryStmts?

- I think the last use of CXXTryStmt is dead in practice: 04c6851cd made it so
  that calls never add edges to try bodies, and the CFG block for a try
  statement is always an empty block containing just the try element itself as
  terminator (the try body itself is part of the normal flow of the function
  and not connected to the block for the try statement itself. The try statment
  cfg block is only connected to the catch bodies, and only reachable from
  throw expressions within the try body.)

- The first use of CXXTryStmt might be important. It looks similar to
  the code that adds all cfg blocks for try statements as roots of
  the reachability graph for the reachability warnings, but I can't
  find a way to trigger it. So I'm omitting it for now. The CXXTryStmt
  code path seems to only be hit by try statements that are function
  bodies without a surrounding compound statements
  (`f() try { ... } catch ...`), and those don't exist for ObjC
  @try statements.
)

Fixes PR52473.

Differential Revfision: https://reviews.llvm.org/D114660

Added: 
clang/test/SemaObjC/return-noreturn.m

Modified: 
clang/lib/Sema/AnalysisBasedWarnings.cpp

Removed: 




diff  --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp 
b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 8544a4fccf4c8..b4dcc9759b990 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -464,7 +464,7 @@ static ControlFlowKind CheckFallThrough(AnalysisDeclContext 
&AC) {
 // No more CFGElements in the block?
 if (ri == re) {
   const Stmt *Term = B.getTerminatorStmt();
-  if (Term && isa(Term)) {
+  if (Term && (isa(Term) || isa(Term))) {
 HasAbnormalEdge = true;
 continue;
   }

diff  --git a/clang/test/SemaObjC/return-noreturn.m 
b/clang/test/SemaObjC/return-noreturn.m
new file mode 100644
index 0..c7735ca211e6d
--- /dev/null
+++ b/clang/test/SemaObjC/return-noreturn.m
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -fsyntax-only -fobjc-exceptions -verify -Wreturn-type 
-Wmissing-noreturn
+
+id f(id self) {
+} // expected-warning {{non-void function does not return a value}}
+
+id f2(id self) {
+  @try {
+@throw (id)0;
+  } @catch (id) {
+  }
+  return (id)0;
+}
+



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


[PATCH] D114677: [AArch64] Avoid crashing on invalid -Wa,-march= values

2021-11-28 Thread Dimitry Andric via Phabricator via cfe-commits
dim created this revision.
dim added reviewers: jcai19, ostannard, DavidSpickett, nickdesaulniers.
Herald added subscribers: kristof.beyls, krytarowski, arichardson.
dim requested review of this revision.
Herald added a project: clang.

As reported in https://bugs.freebsd.org/260078, the gnutls Makefiles
pass -Wa,-march=all to compile a number of assembly files. Clang does
not support this -march value, but because of a mistake in handling
the arguments, an unitialized Arg pointer is dereferenced, which can
cause a segfault.

Work around this by adding a check if the local WaMArch variable is
initialized, and if so, using its value in the diagnostic message.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114677

Files:
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/test/Driver/aarch64-target-as-march.s


Index: clang/test/Driver/aarch64-target-as-march.s
===
--- clang/test/Driver/aarch64-target-as-march.s
+++ clang/test/Driver/aarch64-target-as-march.s
@@ -44,3 +44,12 @@
 // TARGET-FEATURE-3-NOT: "-target-feature" "+v8.4a"
 // TARGET-FEATURE-4: "-target-feature" "+v8.4a"
 // TARGET-FEATURE-4-NOT: "-target-feature" "+v8.3a"
+
+// Invalid -march settings
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=all %s 2>&1 | 
\
+// RUN: FileCheck --check-prefix=INVALID-ARCH-1 %s
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=foobar %s 
2>&1 | \
+// RUN: FileCheck --check-prefix=INVALID-ARCH-2 %s
+
+// INVALID-ARCH-1: error: the clang compiler does not support '-march=all'
+// INVALID-ARCH-2: error: the clang compiler does not support '-march=foobar'
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -260,7 +260,8 @@
 D, getAArch64TargetCPU(Args, Triple, A), Args, Features);
 
   if (!success)
-D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
+D.Diag(diag::err_drv_clang_unsupported)
+<< (WaMArch.size() ? "-march=" + WaMArch.str() : A->getAsString(Args));
 
   if (Args.getLastArg(options::OPT_mgeneral_regs_only)) {
 Features.push_back("-fp-armv8");


Index: clang/test/Driver/aarch64-target-as-march.s
===
--- clang/test/Driver/aarch64-target-as-march.s
+++ clang/test/Driver/aarch64-target-as-march.s
@@ -44,3 +44,12 @@
 // TARGET-FEATURE-3-NOT: "-target-feature" "+v8.4a"
 // TARGET-FEATURE-4: "-target-feature" "+v8.4a"
 // TARGET-FEATURE-4-NOT: "-target-feature" "+v8.3a"
+
+// Invalid -march settings
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=all %s 2>&1 | \
+// RUN: FileCheck --check-prefix=INVALID-ARCH-1 %s
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=foobar %s 2>&1 | \
+// RUN: FileCheck --check-prefix=INVALID-ARCH-2 %s
+
+// INVALID-ARCH-1: error: the clang compiler does not support '-march=all'
+// INVALID-ARCH-2: error: the clang compiler does not support '-march=foobar'
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -260,7 +260,8 @@
 D, getAArch64TargetCPU(Args, Triple, A), Args, Features);
 
   if (!success)
-D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
+D.Diag(diag::err_drv_clang_unsupported)
+<< (WaMArch.size() ? "-march=" + WaMArch.str() : A->getAsString(Args));
 
   if (Args.getLastArg(options::OPT_mgeneral_regs_only)) {
 Features.push_back("-fp-armv8");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D114677: [AArch64] Avoid crashing on invalid -Wa,-march= values

2021-11-28 Thread Thorsten via Phabricator via cfe-commits
tschuett added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:263
   if (!success)
-D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
+D.Diag(diag::err_drv_clang_unsupported)
+<< (WaMArch.size() ? "-march=" + WaMArch.str() : A->getAsString(Args));

Would an if statement be more readable and actually show what the issue is?

Bonus points for a comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114677

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


[PATCH] D114677: [AArch64] Avoid crashing on invalid -Wa,-march= values

2021-11-28 Thread Thorsten via Phabricator via cfe-commits
tschuett added a comment.

There is precedence for checking `WaMArch.size()` in the file. I would expected 
`!WaMArch.empty()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114677

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


[PATCH] D114677: [AArch64] Avoid crashing on invalid -Wa,-march= values

2021-11-28 Thread Dimitry Andric via Phabricator via cfe-commits
dim updated this revision to Diff 390218.
dim added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114677

Files:
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/test/Driver/aarch64-target-as-march.s


Index: clang/test/Driver/aarch64-target-as-march.s
===
--- clang/test/Driver/aarch64-target-as-march.s
+++ clang/test/Driver/aarch64-target-as-march.s
@@ -44,3 +44,12 @@
 // TARGET-FEATURE-3-NOT: "-target-feature" "+v8.4a"
 // TARGET-FEATURE-4: "-target-feature" "+v8.4a"
 // TARGET-FEATURE-4-NOT: "-target-feature" "+v8.3a"
+
+// Invalid -march settings
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=all %s 2>&1 | 
\
+// RUN: FileCheck --check-prefix=INVALID-ARCH-1 %s
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=foobar %s 
2>&1 | \
+// RUN: FileCheck --check-prefix=INVALID-ARCH-2 %s
+
+// INVALID-ARCH-1: error: the clang compiler does not support '-march=all'
+// INVALID-ARCH-2: error: the clang compiler does not support '-march=foobar'
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -225,7 +225,7 @@
   bool success = true;
   // Enable NEON by default.
   Features.push_back("+neon");
-  llvm::StringRef WaMArch = "";
+  llvm::StringRef WaMArch;
   if (ForAS)
 for (const auto *A :
  Args.filtered(options::OPT_Wa_COMMA, options::OPT_Xassembler))
@@ -235,7 +235,7 @@
   // Call getAArch64ArchFeaturesFromMarch only if "-Wa,-march=" or
   // "-Xassembler -march" is detected. Otherwise it may return false
   // and causes Clang to error out.
-  if (WaMArch.size())
+  if (!WaMArch.empty())
 success = getAArch64ArchFeaturesFromMarch(D, WaMArch, Args, Features);
   else if ((A = Args.getLastArg(options::OPT_march_EQ)))
 success = getAArch64ArchFeaturesFromMarch(D, A->getValue(), Args, 
Features);
@@ -259,8 +259,15 @@
 success = getAArch64MicroArchFeaturesFromMcpu(
 D, getAArch64TargetCPU(Args, Triple, A), Args, Features);
 
-  if (!success)
-D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
+  if (!success) {
+auto Diag = D.Diag(diag::err_drv_clang_unsupported);
+// If "-Wa,-march=" is used, 'WaMArch' will contain the argument's value,
+// while 'A' is uninitialized. Only dereference 'A' in the other case.
+if (!WaMArch.empty())
+  Diag << "-march=" + WaMArch.str();
+else
+  Diag << A->getAsString(Args);
+  }
 
   if (Args.getLastArg(options::OPT_mgeneral_regs_only)) {
 Features.push_back("-fp-armv8");


Index: clang/test/Driver/aarch64-target-as-march.s
===
--- clang/test/Driver/aarch64-target-as-march.s
+++ clang/test/Driver/aarch64-target-as-march.s
@@ -44,3 +44,12 @@
 // TARGET-FEATURE-3-NOT: "-target-feature" "+v8.4a"
 // TARGET-FEATURE-4: "-target-feature" "+v8.4a"
 // TARGET-FEATURE-4-NOT: "-target-feature" "+v8.3a"
+
+// Invalid -march settings
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=all %s 2>&1 | \
+// RUN: FileCheck --check-prefix=INVALID-ARCH-1 %s
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=foobar %s 2>&1 | \
+// RUN: FileCheck --check-prefix=INVALID-ARCH-2 %s
+
+// INVALID-ARCH-1: error: the clang compiler does not support '-march=all'
+// INVALID-ARCH-2: error: the clang compiler does not support '-march=foobar'
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -225,7 +225,7 @@
   bool success = true;
   // Enable NEON by default.
   Features.push_back("+neon");
-  llvm::StringRef WaMArch = "";
+  llvm::StringRef WaMArch;
   if (ForAS)
 for (const auto *A :
  Args.filtered(options::OPT_Wa_COMMA, options::OPT_Xassembler))
@@ -235,7 +235,7 @@
   // Call getAArch64ArchFeaturesFromMarch only if "-Wa,-march=" or
   // "-Xassembler -march" is detected. Otherwise it may return false
   // and causes Clang to error out.
-  if (WaMArch.size())
+  if (!WaMArch.empty())
 success = getAArch64ArchFeaturesFromMarch(D, WaMArch, Args, Features);
   else if ((A = Args.getLastArg(options::OPT_march_EQ)))
 success = getAArch64ArchFeaturesFromMarch(D, A->getValue(), Args, Features);
@@ -259,8 +259,15 @@
 success = getAArch64MicroArchFeaturesFromMcpu(
 D, getAArch64TargetCPU(Args, Triple, A), Args, Features);
 
-  if (!success)
-D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
+  if (!success) {
+auto Diag = D.Diag(diag::err_drv_clang_unsupported);
+// If "-Wa,-march=" is used, 'WaMArch' will contain the argum

[PATCH] D114677: [AArch64] Avoid crashing on invalid -Wa,-march= values

2021-11-28 Thread Thorsten via Phabricator via cfe-commits
tschuett accepted this revision.
tschuett added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114677

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


[clang] df08b2f - [AArch64] Avoid crashing on invalid -Wa, -march= values

2021-11-28 Thread Dimitry Andric via cfe-commits

Author: Dimitry Andric
Date: 2021-11-28T22:23:42+01:00
New Revision: df08b2fe8b35cb63dfb3b49738a3494b9b4e6f8e

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

LOG: [AArch64] Avoid crashing on invalid -Wa,-march= values

As reported in https://bugs.freebsd.org/260078, the gnutls Makefiles
pass -Wa,-march=all to compile a number of assembly files. Clang does
not support this -march value, but because of a mistake in handling
the arguments, an unitialized Arg pointer is dereferenced, which can
cause a segfault.

Work around this by adding a check if the local WaMArch variable is
initialized, and if so, using its value in the diagnostic message.

Reviewed By: tschuett

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Arch/AArch64.cpp
clang/test/Driver/aarch64-target-as-march.s

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp 
b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
index 0b60d097b9ca3..abc32f22d2a1b 100644
--- a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -225,7 +225,7 @@ void aarch64::getAArch64TargetFeatures(const Driver &D,
   bool success = true;
   // Enable NEON by default.
   Features.push_back("+neon");
-  llvm::StringRef WaMArch = "";
+  llvm::StringRef WaMArch;
   if (ForAS)
 for (const auto *A :
  Args.filtered(options::OPT_Wa_COMMA, options::OPT_Xassembler))
@@ -235,7 +235,7 @@ void aarch64::getAArch64TargetFeatures(const Driver &D,
   // Call getAArch64ArchFeaturesFromMarch only if "-Wa,-march=" or
   // "-Xassembler -march" is detected. Otherwise it may return false
   // and causes Clang to error out.
-  if (WaMArch.size())
+  if (!WaMArch.empty())
 success = getAArch64ArchFeaturesFromMarch(D, WaMArch, Args, Features);
   else if ((A = Args.getLastArg(options::OPT_march_EQ)))
 success = getAArch64ArchFeaturesFromMarch(D, A->getValue(), Args, 
Features);
@@ -259,8 +259,15 @@ void aarch64::getAArch64TargetFeatures(const Driver &D,
 success = getAArch64MicroArchFeaturesFromMcpu(
 D, getAArch64TargetCPU(Args, Triple, A), Args, Features);
 
-  if (!success)
-D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
+  if (!success) {
+auto Diag = D.Diag(diag::err_drv_clang_unsupported);
+// If "-Wa,-march=" is used, 'WaMArch' will contain the argument's value,
+// while 'A' is uninitialized. Only dereference 'A' in the other case.
+if (!WaMArch.empty())
+  Diag << "-march=" + WaMArch.str();
+else
+  Diag << A->getAsString(Args);
+  }
 
   if (Args.getLastArg(options::OPT_mgeneral_regs_only)) {
 Features.push_back("-fp-armv8");

diff  --git a/clang/test/Driver/aarch64-target-as-march.s 
b/clang/test/Driver/aarch64-target-as-march.s
index a9301ade43351..03c3e395230df 100644
--- a/clang/test/Driver/aarch64-target-as-march.s
+++ b/clang/test/Driver/aarch64-target-as-march.s
@@ -44,3 +44,12 @@
 // TARGET-FEATURE-3-NOT: "-target-feature" "+v8.4a"
 // TARGET-FEATURE-4: "-target-feature" "+v8.4a"
 // TARGET-FEATURE-4-NOT: "-target-feature" "+v8.3a"
+
+// Invalid -march settings
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=all %s 2>&1 | 
\
+// RUN: FileCheck --check-prefix=INVALID-ARCH-1 %s
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=foobar %s 
2>&1 | \
+// RUN: FileCheck --check-prefix=INVALID-ARCH-2 %s
+
+// INVALID-ARCH-1: error: the clang compiler does not support '-march=all'
+// INVALID-ARCH-2: error: the clang compiler does not support '-march=foobar'



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


[PATCH] D114677: [AArch64] Avoid crashing on invalid -Wa,-march= values

2021-11-28 Thread Dimitry Andric 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 rGdf08b2fe8b35: [AArch64] Avoid crashing on invalid 
-Wa,-march= values (authored by dim).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114677

Files:
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/test/Driver/aarch64-target-as-march.s


Index: clang/test/Driver/aarch64-target-as-march.s
===
--- clang/test/Driver/aarch64-target-as-march.s
+++ clang/test/Driver/aarch64-target-as-march.s
@@ -44,3 +44,12 @@
 // TARGET-FEATURE-3-NOT: "-target-feature" "+v8.4a"
 // TARGET-FEATURE-4: "-target-feature" "+v8.4a"
 // TARGET-FEATURE-4-NOT: "-target-feature" "+v8.3a"
+
+// Invalid -march settings
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=all %s 2>&1 | 
\
+// RUN: FileCheck --check-prefix=INVALID-ARCH-1 %s
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=foobar %s 
2>&1 | \
+// RUN: FileCheck --check-prefix=INVALID-ARCH-2 %s
+
+// INVALID-ARCH-1: error: the clang compiler does not support '-march=all'
+// INVALID-ARCH-2: error: the clang compiler does not support '-march=foobar'
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -225,7 +225,7 @@
   bool success = true;
   // Enable NEON by default.
   Features.push_back("+neon");
-  llvm::StringRef WaMArch = "";
+  llvm::StringRef WaMArch;
   if (ForAS)
 for (const auto *A :
  Args.filtered(options::OPT_Wa_COMMA, options::OPT_Xassembler))
@@ -235,7 +235,7 @@
   // Call getAArch64ArchFeaturesFromMarch only if "-Wa,-march=" or
   // "-Xassembler -march" is detected. Otherwise it may return false
   // and causes Clang to error out.
-  if (WaMArch.size())
+  if (!WaMArch.empty())
 success = getAArch64ArchFeaturesFromMarch(D, WaMArch, Args, Features);
   else if ((A = Args.getLastArg(options::OPT_march_EQ)))
 success = getAArch64ArchFeaturesFromMarch(D, A->getValue(), Args, 
Features);
@@ -259,8 +259,15 @@
 success = getAArch64MicroArchFeaturesFromMcpu(
 D, getAArch64TargetCPU(Args, Triple, A), Args, Features);
 
-  if (!success)
-D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
+  if (!success) {
+auto Diag = D.Diag(diag::err_drv_clang_unsupported);
+// If "-Wa,-march=" is used, 'WaMArch' will contain the argument's value,
+// while 'A' is uninitialized. Only dereference 'A' in the other case.
+if (!WaMArch.empty())
+  Diag << "-march=" + WaMArch.str();
+else
+  Diag << A->getAsString(Args);
+  }
 
   if (Args.getLastArg(options::OPT_mgeneral_regs_only)) {
 Features.push_back("-fp-armv8");


Index: clang/test/Driver/aarch64-target-as-march.s
===
--- clang/test/Driver/aarch64-target-as-march.s
+++ clang/test/Driver/aarch64-target-as-march.s
@@ -44,3 +44,12 @@
 // TARGET-FEATURE-3-NOT: "-target-feature" "+v8.4a"
 // TARGET-FEATURE-4: "-target-feature" "+v8.4a"
 // TARGET-FEATURE-4-NOT: "-target-feature" "+v8.3a"
+
+// Invalid -march settings
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=all %s 2>&1 | \
+// RUN: FileCheck --check-prefix=INVALID-ARCH-1 %s
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=foobar %s 2>&1 | \
+// RUN: FileCheck --check-prefix=INVALID-ARCH-2 %s
+
+// INVALID-ARCH-1: error: the clang compiler does not support '-march=all'
+// INVALID-ARCH-2: error: the clang compiler does not support '-march=foobar'
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -225,7 +225,7 @@
   bool success = true;
   // Enable NEON by default.
   Features.push_back("+neon");
-  llvm::StringRef WaMArch = "";
+  llvm::StringRef WaMArch;
   if (ForAS)
 for (const auto *A :
  Args.filtered(options::OPT_Wa_COMMA, options::OPT_Xassembler))
@@ -235,7 +235,7 @@
   // Call getAArch64ArchFeaturesFromMarch only if "-Wa,-march=" or
   // "-Xassembler -march" is detected. Otherwise it may return false
   // and causes Clang to error out.
-  if (WaMArch.size())
+  if (!WaMArch.empty())
 success = getAArch64ArchFeaturesFromMarch(D, WaMArch, Args, Features);
   else if ((A = Args.getLastArg(options::OPT_march_EQ)))
 success = getAArch64ArchFeaturesFromMarch(D, A->getValue(), Args, Features);
@@ -259,8 +259,15 @@
 success = getAArch64MicroArchFeaturesFromMcpu(
 D, getAArch64TargetCPU(Args, Triple, A), Args, Features);
 
-  if (!success)
-D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Ar

[PATCH] D113995: [clangd] Dex Trigrams: Improve query trigram generation

2021-11-28 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/index/dex/Trigram.cpp:71
   }
-  // Emit short-query trigrams: FooBar -> f, fo, fb.
-  if (!LowercaseIdentifier.empty())
-Out(Trigram(LowercaseIdentifier[0]));
-  if (LowercaseIdentifier.size() >= 2)
-Out(Trigram(LowercaseIdentifier[0], LowercaseIdentifier[1]));
-  for (size_t I = 1; I < LowercaseIdentifier.size(); ++I)
-if (Roles[I] == Head) {
-  Out(Trigram(LowercaseIdentifier[0], LowercaseIdentifier[I]));
+  // Short queries semantics are different: emit incomplete trigrams for them.
+  //

It'd be good to describe what the semantics actually are somewhere.
As it is, this comment describes the algorithm used to generate the trigrams, 
and the other function refers to this one.



Comment at: clang-tools-extra/clangd/index/dex/Trigram.cpp:74
+  // First, the first separator if it is preceding the alphanumeric symbols.
+  size_t FirstSeparator = std::numeric_limits::max();
+  for (size_t I = 0; I < LowercaseIdentifier.size(); ++I) {

This seems overly complicated.
If a separator precedes the alphanums, then it's at the front.
Moreover its Next table seems to be set up correctly.

So I believe this all amounts to: if the first char is a separator, then we 
allow starting our partial trigrams at 0, first head, second head. Else just 
first head & second head. And we just follow the next table.



Comment at: clang-tools-extra/clangd/index/dex/Trigram.cpp:87
+  //
+  // - First head
+  // - Fist head + subsequent tail

This looks a lot like the Next table. Can't we use that? (Second head + third 
head is missing, but I don't see why)



Comment at: clang-tools-extra/clangd/index/dex/Trigram.cpp:147
+
+  unsigned ValidSymbols =
+  llvm::count_if(Roles, [](CharRole R) { return R == Head || R == Tail; });

both "valid" and "symbols" are confusing here.

Do we actually need this variable?
Seems like we can detect the case at the end, if we generated no trigrams.



Comment at: clang-tools-extra/clangd/index/dex/Trigram.cpp:152
+  if (ValidSymbols < 3) {
+// If a bigram can't be formed, we might prepend a separator.
+std::string Letters = Roles.front() == Separator && ValidSymbols < 2

I'd suggest always including the separator, and then using 
StringRef(Letters).take_back(2). Then we don't need ValidSymbols here either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113995

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


[PATCH] D105869: [Driver] fix PowerPC SPE musl dynamic linker name

2021-11-28 Thread Patrick Oppenlander via Phabricator via cfe-commits
pattop updated this revision to Diff 390227.
pattop added a comment.

Add test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105869

Files:
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/Driver/linux-ld.c


Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -1718,6 +1718,9 @@
 // RUN: --target=powerpc64-pc-linux-musl \
 // RUN:   | FileCheck --check-prefix=CHECK-MUSL-PPC64 %s
 // RUN: %clang %s -### -o %t.o 2>&1 \
+// RUN: --target=powerpcspe-pc-linux-musl \
+// RUN:   | FileCheck --check-prefix=CHECK-MUSL-PPCSPE %s
+// RUN: %clang %s -### -o %t.o 2>&1 \
 // RUN: --target=thumb-pc-linux-musleabi \
 // RUN:   | FileCheck --check-prefix=CHECK-MUSL-ARM %s
 // RUN: %clang %s -### -o %t.o 2>&1 \
@@ -1767,6 +1770,7 @@
 // CHECK-MUSL-MIPS64EL:   "-dynamic-linker" "/lib/ld-musl-mips64el.so.1"
 // CHECK-MUSL-PPC:"-dynamic-linker" "/lib/ld-musl-powerpc.so.1"
 // CHECK-MUSL-PPC64:  "-dynamic-linker" "/lib/ld-musl-powerpc64.so.1"
+// CHECK-MUSL-PPCSPE: "-dynamic-linker" "/lib/ld-musl-powerpc-sf.so.1"
 // CHECK-MUSL-ARM:"-dynamic-linker" "/lib/ld-musl-arm.so.1"
 // CHECK-MUSL-ARMHF:  "-dynamic-linker" "/lib/ld-musl-armhf.so.1"
 // CHECK-MUSL-ARMEB:  "-dynamic-linker" "/lib/ld-musl-armeb.so.1"
Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -421,6 +421,9 @@
 (Triple.getEnvironment() == llvm::Triple::MuslEABIHF ||
  tools::arm::getARMFloatABI(*this, Args) == 
tools::arm::FloatABI::Hard))
   ArchName += "hf";
+if (Arch == llvm::Triple::ppc &&
+Triple.getSubArch() == llvm::Triple::PPCSubArch_spe)
+  ArchName = "powerpc-sf";
 
 return "/lib/ld-musl-" + ArchName + ".so.1";
   }


Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -1718,6 +1718,9 @@
 // RUN: --target=powerpc64-pc-linux-musl \
 // RUN:   | FileCheck --check-prefix=CHECK-MUSL-PPC64 %s
 // RUN: %clang %s -### -o %t.o 2>&1 \
+// RUN: --target=powerpcspe-pc-linux-musl \
+// RUN:   | FileCheck --check-prefix=CHECK-MUSL-PPCSPE %s
+// RUN: %clang %s -### -o %t.o 2>&1 \
 // RUN: --target=thumb-pc-linux-musleabi \
 // RUN:   | FileCheck --check-prefix=CHECK-MUSL-ARM %s
 // RUN: %clang %s -### -o %t.o 2>&1 \
@@ -1767,6 +1770,7 @@
 // CHECK-MUSL-MIPS64EL:   "-dynamic-linker" "/lib/ld-musl-mips64el.so.1"
 // CHECK-MUSL-PPC:"-dynamic-linker" "/lib/ld-musl-powerpc.so.1"
 // CHECK-MUSL-PPC64:  "-dynamic-linker" "/lib/ld-musl-powerpc64.so.1"
+// CHECK-MUSL-PPCSPE: "-dynamic-linker" "/lib/ld-musl-powerpc-sf.so.1"
 // CHECK-MUSL-ARM:"-dynamic-linker" "/lib/ld-musl-arm.so.1"
 // CHECK-MUSL-ARMHF:  "-dynamic-linker" "/lib/ld-musl-armhf.so.1"
 // CHECK-MUSL-ARMEB:  "-dynamic-linker" "/lib/ld-musl-armeb.so.1"
Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -421,6 +421,9 @@
 (Triple.getEnvironment() == llvm::Triple::MuslEABIHF ||
  tools::arm::getARMFloatABI(*this, Args) == tools::arm::FloatABI::Hard))
   ArchName += "hf";
+if (Arch == llvm::Triple::ppc &&
+Triple.getSubArch() == llvm::Triple::PPCSubArch_spe)
+  ArchName = "powerpc-sf";
 
 return "/lib/ld-musl-" + ArchName + ".so.1";
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105869: [Driver] fix PowerPC SPE musl dynamic linker name

2021-11-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

I have confirmed that this matches musl `#define FP_SUFFIX "-sf"`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105869

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


[clang] b3163c1 - [Driver] Support PowerPC SPE musl dynamic linker name ld-musl-powerpc-sf.so.1

2021-11-28 Thread Fangrui Song via cfe-commits

Author: Patrick Oppenlander
Date: 2021-11-28T15:39:55-08:00
New Revision: b3163c1cdde09467382984e5879aa9701b7036d3

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

LOG: [Driver] Support PowerPC SPE musl dynamic linker name 
ld-musl-powerpc-sf.so.1

Musl treats PowerPC SPE as a soft-float target (as the PowerPC SPE ABI
is soft-float compatible).

Reviewed By: jhibbits, MaskRay

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Linux.cpp
clang/test/Driver/linux-ld.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Linux.cpp 
b/clang/lib/Driver/ToolChains/Linux.cpp
index 0224383e63a1a..198774506e5e5 100644
--- a/clang/lib/Driver/ToolChains/Linux.cpp
+++ b/clang/lib/Driver/ToolChains/Linux.cpp
@@ -421,6 +421,9 @@ std::string Linux::getDynamicLinker(const ArgList &Args) 
const {
 (Triple.getEnvironment() == llvm::Triple::MuslEABIHF ||
  tools::arm::getARMFloatABI(*this, Args) == 
tools::arm::FloatABI::Hard))
   ArchName += "hf";
+if (Arch == llvm::Triple::ppc &&
+Triple.getSubArch() == llvm::Triple::PPCSubArch_spe)
+  ArchName = "powerpc-sf";
 
 return "/lib/ld-musl-" + ArchName + ".so.1";
   }

diff  --git a/clang/test/Driver/linux-ld.c b/clang/test/Driver/linux-ld.c
index 89fd22f7e8c60..31905e1a3d00b 100644
--- a/clang/test/Driver/linux-ld.c
+++ b/clang/test/Driver/linux-ld.c
@@ -1718,6 +1718,9 @@
 // RUN: --target=powerpc64-pc-linux-musl \
 // RUN:   | FileCheck --check-prefix=CHECK-MUSL-PPC64 %s
 // RUN: %clang %s -### -o %t.o 2>&1 \
+// RUN: --target=powerpcspe-pc-linux-musl \
+// RUN:   | FileCheck --check-prefix=CHECK-MUSL-PPCSPE %s
+// RUN: %clang %s -### -o %t.o 2>&1 \
 // RUN: --target=thumb-pc-linux-musleabi \
 // RUN:   | FileCheck --check-prefix=CHECK-MUSL-ARM %s
 // RUN: %clang %s -### -o %t.o 2>&1 \
@@ -1767,6 +1770,7 @@
 // CHECK-MUSL-MIPS64EL:   "-dynamic-linker" "/lib/ld-musl-mips64el.so.1"
 // CHECK-MUSL-PPC:"-dynamic-linker" "/lib/ld-musl-powerpc.so.1"
 // CHECK-MUSL-PPC64:  "-dynamic-linker" "/lib/ld-musl-powerpc64.so.1"
+// CHECK-MUSL-PPCSPE: "-dynamic-linker" "/lib/ld-musl-powerpc-sf.so.1"
 // CHECK-MUSL-ARM:"-dynamic-linker" "/lib/ld-musl-arm.so.1"
 // CHECK-MUSL-ARMHF:  "-dynamic-linker" "/lib/ld-musl-armhf.so.1"
 // CHECK-MUSL-ARMEB:  "-dynamic-linker" "/lib/ld-musl-armeb.so.1"



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


[PATCH] D105869: [Driver] Support PowerPC SPE musl dynamic linker name ld-musl-powerpc-sf.so.1

2021-11-28 Thread Fangrui Song 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 rGb3163c1cdde0: [Driver] Support PowerPC SPE musl dynamic 
linker name ld-musl-powerpc-sf.so.1 (authored by pattop, committed by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105869

Files:
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/Driver/linux-ld.c


Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -1718,6 +1718,9 @@
 // RUN: --target=powerpc64-pc-linux-musl \
 // RUN:   | FileCheck --check-prefix=CHECK-MUSL-PPC64 %s
 // RUN: %clang %s -### -o %t.o 2>&1 \
+// RUN: --target=powerpcspe-pc-linux-musl \
+// RUN:   | FileCheck --check-prefix=CHECK-MUSL-PPCSPE %s
+// RUN: %clang %s -### -o %t.o 2>&1 \
 // RUN: --target=thumb-pc-linux-musleabi \
 // RUN:   | FileCheck --check-prefix=CHECK-MUSL-ARM %s
 // RUN: %clang %s -### -o %t.o 2>&1 \
@@ -1767,6 +1770,7 @@
 // CHECK-MUSL-MIPS64EL:   "-dynamic-linker" "/lib/ld-musl-mips64el.so.1"
 // CHECK-MUSL-PPC:"-dynamic-linker" "/lib/ld-musl-powerpc.so.1"
 // CHECK-MUSL-PPC64:  "-dynamic-linker" "/lib/ld-musl-powerpc64.so.1"
+// CHECK-MUSL-PPCSPE: "-dynamic-linker" "/lib/ld-musl-powerpc-sf.so.1"
 // CHECK-MUSL-ARM:"-dynamic-linker" "/lib/ld-musl-arm.so.1"
 // CHECK-MUSL-ARMHF:  "-dynamic-linker" "/lib/ld-musl-armhf.so.1"
 // CHECK-MUSL-ARMEB:  "-dynamic-linker" "/lib/ld-musl-armeb.so.1"
Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -421,6 +421,9 @@
 (Triple.getEnvironment() == llvm::Triple::MuslEABIHF ||
  tools::arm::getARMFloatABI(*this, Args) == 
tools::arm::FloatABI::Hard))
   ArchName += "hf";
+if (Arch == llvm::Triple::ppc &&
+Triple.getSubArch() == llvm::Triple::PPCSubArch_spe)
+  ArchName = "powerpc-sf";
 
 return "/lib/ld-musl-" + ArchName + ".so.1";
   }


Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -1718,6 +1718,9 @@
 // RUN: --target=powerpc64-pc-linux-musl \
 // RUN:   | FileCheck --check-prefix=CHECK-MUSL-PPC64 %s
 // RUN: %clang %s -### -o %t.o 2>&1 \
+// RUN: --target=powerpcspe-pc-linux-musl \
+// RUN:   | FileCheck --check-prefix=CHECK-MUSL-PPCSPE %s
+// RUN: %clang %s -### -o %t.o 2>&1 \
 // RUN: --target=thumb-pc-linux-musleabi \
 // RUN:   | FileCheck --check-prefix=CHECK-MUSL-ARM %s
 // RUN: %clang %s -### -o %t.o 2>&1 \
@@ -1767,6 +1770,7 @@
 // CHECK-MUSL-MIPS64EL:   "-dynamic-linker" "/lib/ld-musl-mips64el.so.1"
 // CHECK-MUSL-PPC:"-dynamic-linker" "/lib/ld-musl-powerpc.so.1"
 // CHECK-MUSL-PPC64:  "-dynamic-linker" "/lib/ld-musl-powerpc64.so.1"
+// CHECK-MUSL-PPCSPE: "-dynamic-linker" "/lib/ld-musl-powerpc-sf.so.1"
 // CHECK-MUSL-ARM:"-dynamic-linker" "/lib/ld-musl-arm.so.1"
 // CHECK-MUSL-ARMHF:  "-dynamic-linker" "/lib/ld-musl-armhf.so.1"
 // CHECK-MUSL-ARMEB:  "-dynamic-linker" "/lib/ld-musl-armeb.so.1"
Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -421,6 +421,9 @@
 (Triple.getEnvironment() == llvm::Triple::MuslEABIHF ||
  tools::arm::getARMFloatABI(*this, Args) == tools::arm::FloatABI::Hard))
   ArchName += "hf";
+if (Arch == llvm::Triple::ppc &&
+Triple.getSubArch() == llvm::Triple::PPCSubArch_spe)
+  ArchName = "powerpc-sf";
 
 return "/lib/ld-musl-" + ArchName + ".so.1";
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 5233ad1 - Compilation Database: Point Bazel users to a solution

2021-11-28 Thread Nathan Ridge via cfe-commits

Author: Nathan Ridge
Date: 2021-11-28T19:30:06-05:00
New Revision: 5233ad17e77eb7b7697f2664dc5a588a30fe5eaf

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

LOG: Compilation Database: Point Bazel users to a solution

This doc lists ways of getting compilation databases out of some popular build 
systems for use with clangd and other tooling.

We built such a way for Bazel and just released it after a bunch of requests on 
GitHub. Thought I should propose that we add a link to help people find it and 
use clang tooling.

(This is my first revision submitted via LLVM Phabricator, so if I've messed 
something up, I'd really appreciate your help and patience. Asking for a review 
from Sam McCall, because I've had a great time working with him elsewhere on 
GitHub before and because I saw him in the Git history for this file.)

Reviewed By: sammccall

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

Added: 


Modified: 
clang/docs/JSONCompilationDatabase.rst

Removed: 




diff  --git a/clang/docs/JSONCompilationDatabase.rst 
b/clang/docs/JSONCompilationDatabase.rst
index bd91db2583980..fd822c78d1fe9 100644
--- a/clang/docs/JSONCompilationDatabase.rst
+++ b/clang/docs/JSONCompilationDatabase.rst
@@ -36,6 +36,12 @@ works) with the option ``CMAKE_EXPORT_COMPILE_COMMANDS``.
 For projects on Linux, there is an alternative to intercept compiler
 calls with a tool called `Bear `_.
 
+`Bazel `_ can export a compilation database via 
+`this extractor extension 
+`_.
+Bazel is otherwise resistant to Bear and other compiler-intercept
+techniques.
+
 Clang's tooling interface supports reading compilation databases; see
 the :doc:`LibTooling documentation `. libclang and its
 python bindings also support this (since clang 3.2); see



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


[PATCH] D114213: Compilation Database: Point Bazel users to a solution

2021-11-28 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5233ad17e77e: Compilation Database: Point Bazel users to a 
solution (authored by nridge).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114213

Files:
  clang/docs/JSONCompilationDatabase.rst


Index: clang/docs/JSONCompilationDatabase.rst
===
--- clang/docs/JSONCompilationDatabase.rst
+++ clang/docs/JSONCompilationDatabase.rst
@@ -36,6 +36,12 @@
 For projects on Linux, there is an alternative to intercept compiler
 calls with a tool called `Bear `_.
 
+`Bazel `_ can export a compilation database via 
+`this extractor extension 
+`_.
+Bazel is otherwise resistant to Bear and other compiler-intercept
+techniques.
+
 Clang's tooling interface supports reading compilation databases; see
 the :doc:`LibTooling documentation `. libclang and its
 python bindings also support this (since clang 3.2); see


Index: clang/docs/JSONCompilationDatabase.rst
===
--- clang/docs/JSONCompilationDatabase.rst
+++ clang/docs/JSONCompilationDatabase.rst
@@ -36,6 +36,12 @@
 For projects on Linux, there is an alternative to intercept compiler
 calls with a tool called `Bear `_.
 
+`Bazel `_ can export a compilation database via 
+`this extractor extension 
+`_.
+Bazel is otherwise resistant to Bear and other compiler-intercept
+techniques.
+
 Clang's tooling interface supports reading compilation databases; see
 the :doc:`LibTooling documentation `. libclang and its
 python bindings also support this (since clang 3.2); see
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D114213: Compilation Database: Point Bazel users to a solution

2021-11-28 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment.

In D114213#3156940 , @cpsauer wrote:

> could I ask for your help landing the change now that it's approved?

Landed!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114213

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


[PATCH] D114213: Compilation Database: Point Bazel users to a solution

2021-11-28 Thread Christopher Sauer via Phabricator via cfe-commits
cpsauer added a comment.

Wahoo! Thank you


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114213

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


[clang] 4aac00a - [analyzer][doc] Add user documenation for taint analysis

2021-11-28 Thread Endre Fülöp via cfe-commits

Author: Endre Fülöp
Date: 2021-11-28T23:36:47+01:00
New Revision: 4aac00a71db31121d70b140d7367e7f9d9992f66

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

LOG: [analyzer][doc] Add user documenation for taint analysis

Checker alpha.security.taint.TaintPropagation now has user documentation for
taint analysis with an example showing external YAML configuration format.
The format of the taint configuration file is now documented under the user
documentation of Clang SA.

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

Added: 
clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst

Modified: 
clang/docs/analyzer/checkers.rst
clang/docs/analyzer/user-docs.rst

Removed: 




diff  --git a/clang/docs/analyzer/checkers.rst 
b/clang/docs/analyzer/checkers.rst
index df62fb0643f86..a31c38c133d97 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -2317,8 +2317,15 @@ Checkers implementing `taint analysis 
`_ format. The 
taint-related options defined in the config file extend but do not override the 
built-in sources, rules, sinks.
+The format of the external taint configuration file is not stable, and could 
change without any notice even in a non-backward compatible way.
+
+For a more detailed description of configuration options, please see the 
:doc:`user-docs/TaintAnalysisConfiguration`. For an example see 
:ref:`clangsa-taint-configuration-example`.
+
 alpha.unix
 ^^^
 

diff  --git a/clang/docs/analyzer/user-docs.rst 
b/clang/docs/analyzer/user-docs.rst
index 69486c52d2873..2292cec6944b1 100644
--- a/clang/docs/analyzer/user-docs.rst
+++ b/clang/docs/analyzer/user-docs.rst
@@ -7,3 +7,4 @@ Contents:
:maxdepth: 2
 
user-docs/CrossTranslationUnit
+   user-docs/TaintAnalysisConfiguration

diff  --git a/clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst 
b/clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst
new file mode 100644
index 0..94db84494e00b
--- /dev/null
+++ b/clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst
@@ -0,0 +1,170 @@
+
+Taint Analysis Configuration
+
+
+The Clang Static Analyzer uses taint analysis to detect security-related 
issues in code.
+The backbone of taint analysis in the Clang SA is the `GenericTaintChecker`, 
which the user can access via the :ref:`alpha-security-taint-TaintPropagation` 
checker alias and this checker has a default taint-related configuration.
+The built-in default settings are defined in code, and they are always in 
effe

[PATCH] D113251: [analyzer][doc] Add user documenation for taint analysis

2021-11-28 Thread Endre Fülöp via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4aac00a71db3: [analyzer][doc] Add user documenation for 
taint analysis (authored by gamesh411).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113251

Files:
  clang/docs/analyzer/checkers.rst
  clang/docs/analyzer/user-docs.rst
  clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst

Index: clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst
===
--- /dev/null
+++ clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst
@@ -0,0 +1,170 @@
+
+Taint Analysis Configuration
+
+
+The Clang Static Analyzer uses taint analysis to detect security-related issues in code.
+The backbone of taint analysis in the Clang SA is the `GenericTaintChecker`, which the user can access via the :ref:`alpha-security-taint-TaintPropagation` checker alias and this checker has a default taint-related configuration.
+The built-in default settings are defined in code, and they are always in effect once the checker is enabled, either directly or via the alias.
+The checker also provides a configuration interface for extending the default settings by providing a configuration file in `YAML `_ format.
+This documentation describes the syntax of the configuration file and gives the informal semantics of the configuration options.
+
+.. contents::
+   :local:
+
+.. _clangsa-taint-configuration-overview:
+
+Overview
+
+
+Taint analysis works by checking for the occurrence of special operations during the symbolic execution of the program.
+Taint analysis defines sources, sinks, and propagation rules. It identifies errors by detecting a flow of information that originates from a taint source, reaches a taint sink, and propagates through the program paths via propagation rules.
+A source, sink, or an operation that propagates taint is mainly domain-specific knowledge, but there are some built-in defaults provided by :ref:`alpha-security-taint-TaintPropagation`.
+It is possible to express that a statement sanitizes tainted values by providing a ``Filters`` section in the external configuration (see :ref:`clangsa-taint-configuration-example` and :ref:`clangsa-taint-filter-details`).
+There are no default filters defined in the built-in settings.
+The checker's documentation also specifies how to provide a custom taint configuration with command-line options.
+
+.. _clangsa-taint-configuration-example:
+
+Example configuration file
+__
+
+.. code-block:: yaml
+
+  # The entries that specify arguments use 0-based indexing when specifying
+  # input arguments, and -1 is used to denote the return value.
+
+  Filters:
+# Filter functions
+# Taint is sanitized when tainted variables are pass arguments to filters.
+
+# Filter function
+#   void cleanse_first_arg(int* arg)
+#
+# Result example:
+#   int x; // x is tainted
+#   cleanse_first_arg(&x); // x is not tainted after the call
+- Name: cleanse_first_arg
+  Args: [0]
+
+  Propagations:
+# Source functions
+# The omission of SrcArgs key indicates unconditional taint propagation,
+# which is conceptually what a source does.
+
+# Source function
+#   size_t fread(void *ptr, size_t size, size_t nmemb, FILE * stream)
+#
+# Result example:
+#   FILE* f = fopen("file.txt");
+#   char buf[1024];
+#   size_t read = fread(buf, sizeof(buf[0]), sizeof(buf)/sizeof(buf[0]), f);
+#   // both read and buf are tainted
+- Name: fread
+  DstArgs: [0, -1]
+
+# Propagation functions
+# The presence of SrcArgs key indicates conditional taint propagation,
+# which is conceptually what a propagator does.
+
+# Propagation function
+#   char *dirname(char *path)
+#
+# Result example:
+#   char* path = read_path();
+#   char* dir = dirname(path);
+#   // dir is tainted if path was tainted
+- Name: dirname
+  SrcArgs: [0]
+  DstArgs: [-1]
+
+  Sinks:
+# Sink functions
+# If taint reaches any of the arguments specified, a warning is emitted.
+
+# Sink function
+#   int system(const char* command)
+#
+# Result example:
+#   const char* command = read_command();
+#   system(command); // emit diagnostic if command is tainted
+- Name: system
+  Args: [0]
+
+In the example file above, the entries under the `Propagation` key implement the conceptual sources and propagations, and sinks have their dedicated `Sinks` key.
+The user can define operations (function calls) where the tainted values should be cleansed by listing entries under the `Filters` key.
+Filters model the

[PATCH] D114688: [Clang] Add __builtin_elementwise_ceil

2021-11-28 Thread Jun Zhang via Phabricator via cfe-commits
junaire created this revision.
junaire added reviewers: fhahn, aaron.ballman, scanon, craig.topper.
junaire requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch implements one of the missing builtin functions specified in 
https://reviews.llvm.org/D111529


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114688

Files:
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins-elementwise-math.c
  clang/test/Sema/builtins-elementwise-math.c
  clang/test/SemaCXX/builtins-elementwise-math.cpp

Index: clang/test/SemaCXX/builtins-elementwise-math.cpp
===
--- clang/test/SemaCXX/builtins-elementwise-math.cpp
+++ clang/test/SemaCXX/builtins-elementwise-math.cpp
@@ -36,3 +36,10 @@
   static_assert(!is_const::value);
   static_assert(!is_const::value);
 }
+
+void test_builtin_elementwise_ceil() {
+  const float a = 42.0;
+  float b = 42.3;
+  static_assert(!is_const::value);
+  static_assert(!is_const::value);
+}
Index: clang/test/Sema/builtins-elementwise-math.c
===
--- clang/test/Sema/builtins-elementwise-math.c
+++ clang/test/Sema/builtins-elementwise-math.c
@@ -135,3 +135,24 @@
   c1 = __builtin_elementwise_min(c1, c2);
   // expected-error@-1 {{1st argument must be a vector, integer or floating point type (was '_Complex float')}}
 }
+
+void test_builtin_elementwise_ceil(int i, float f, double d, float4 v, int3 iv, unsigned u, unsigned4 uv) {
+
+  struct Foo s = __builtin_elementwise_ceil(f);
+  // expected-error@-1 {{initializing 'struct Foo' with an expression of incompatible type 'float'}}
+
+  i = __builtin_elementwise_ceil();
+  // expected-error@-1 {{too few arguments to function call, expected 1, have 0}}
+
+  i = __builtin_elementwise_ceil(i);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'int')}}
+
+  i = __builtin_elementwise_ceil(f, f);
+  // expected-error@-1 {{too many arguments to function call, expected 1, have 2}}
+
+  u = __builtin_elementwise_ceil(u);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'unsigned int')}}
+
+  uv = __builtin_elementwise_ceil(uv);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'unsigned4' (vector of 4 'unsigned int' values))}}
+}
Index: clang/test/CodeGen/builtins-elementwise-math.c
===
--- clang/test/CodeGen/builtins-elementwise-math.c
+++ clang/test/CodeGen/builtins-elementwise-math.c
@@ -189,3 +189,20 @@
   // CHECK-NEXT: call i32 @llvm.smin.i32(i32 [[IAS1]], i32 [[B]])
   int_as_one = __builtin_elementwise_min(int_as_one, b);
 }
+
+void test_builtin_elementwise_ceil(float f1, float f2, double d1, double d2,
+   float4 vf1, float4 vf2, si8 vi1, si8 vi2,
+   long long int i1, long long int i2, short si) {
+  // CHECK-LABEL: define void @test_builtin_elementwise_ceil(
+  // CHECK:  [[F1:%.+]] = load float, float* %f1.addr, align 4
+  // CHECK-NEXT:  call float @llvm.ceil.f32(float [[F1]])
+  f2 = __builtin_elementwise_ceil(f1);
+
+  // CHECK:  [[D1:%.+]] = load double, double* %d1.addr, align 8
+  // CHECK-NEXT: call double @llvm.ceil.f64(double [[D1]])
+  d2 = __builtin_elementwise_ceil(d1);
+
+  // CHECK:  [[VF1:%.+]] = load <4 x float>, <4 x float>* %vf1.addr, align 16
+  // CHECK-NEXT: call <4 x float> @llvm.ceil.v4f32(<4 x float> [[VF1]])
+  vf2 = __builtin_elementwise_ceil(vf1);
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -2102,6 +2102,11 @@
 if (SemaBuiltinElementwiseMathOneArg(TheCall))
   return ExprError();
 break;
+  case Builtin::BI__builtin_elementwise_ceil:
+if (SemaBuiltinElementwiseMathFloatArg(TheCall))
+  return ExprError();
+break;
+
   case Builtin::BI__builtin_elementwise_min:
   case Builtin::BI__builtin_elementwise_max:
 if (SemaBuiltinElementwiseMath(TheCall))
@@ -16713,6 +16718,31 @@
   return false;
 }
 
+bool Sema::SemaBuiltinElementwiseMathFloatArg(CallExpr *TheCall) {
+  if (checkArgCount(*this, TheCall, 1))
+return true;
+
+  ExprResult A = UsualUnaryConversions(TheCall->getArg(0));
+  SourceLocation ArgLoc = TheCall->getArg(0)->getBeginLoc();
+  if (A.isInvalid())
+return true;
+
+  TheCall->setArg(0, A.get());
+  QualType TyA = A.get()->getType();
+  if (checkMathBuiltinElementType(*this, ArgLoc, TyA))
+return true;
+
+  QualType EltTy = TyA;
+  if (auto *VecTy = EltTy->getAs())
+EltTy = VecTy->getElementType();
+  if (EltTy->isUnsignedIntegerType() || EltTy->isInteger

[PATCH] D114639: Raise the minimum Visual Studio version to VS2019

2021-11-28 Thread James Henderson via Phabricator via cfe-commits
jhenderson added a comment.

No more comments from me (apart from one minor nit). This should definitely get 
someone with more familiarity with how these things are configured to take a 
look though.




Comment at: clang/docs/UsersManual.rst:3546
 
-cmake -G"Visual Studio 15 2017" -T LLVM ..
+cmake -G"Visual Studio 17 2022" -T LLVM ..
 

I think the missing space should be fixed to :)



Comment at: llvm/include/llvm/Support/Compiler.h:106-108
 /// Sadly, this is separate from just rvalue reference support because GCC
 /// and MSVC implemented this later than everything else. This appears to be
 /// corrected in MSVC 2019 but not MSVC 2017.

RKSimon wrote:
> jhenderson wrote:
> > Does this comment need changing?
> An even bigger question is - can we get rid of the 
> LLVM_HAS_RVALUE_REFERENCE_THIS define entirely now? Either as part of this 
> patch or as a followup
Yeah, another patch entirely to sort this would be fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114639

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