[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.

Thanks for bearing with me, JF!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65986



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

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

[JF Bastien via Phabricator via cfe-commits]

jfb marked an inline comment as done.
jfb added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:172
+  case VFSMode::Unknown:
+if (!this->VFS) {
+  LLVM_FALLTHROUGH;

JDevlieghere wrote:
> jfb wrote:
> > JDevlieghere wrote:
> > > Is this really necessary? 
> > the `Driver` ctor takes a `VFS` parameter which some tools set (otherwise 
> > it defaults to `nullptr`), so if the command-line `VFS` isn't specified we 
> > want to leave whatever was passed in as-is. If the command line parameter 
> > is set the we'll obey that instead.
> > 
> > So yes I think it's the right thing to do.
> FWIW, I got confused by the indentation of the case element. Even though it's 
> in the if-case it just acts like a label.  While clever, I think for the sake 
> of readability it might be worth just repeating `this->VFS = 
> llvm::vfs::createPhysicalFileSystem().release();`. 
Ah gotcha, updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65986



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

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 214298.
jfb marked an inline comment as done.
jfb added a comment.

- Address more comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65986

Files:
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/vfsmode.py

Index: clang/test/Driver/vfsmode.py
===
--- /dev/null
+++ clang/test/Driver/vfsmode.py
@@ -0,0 +1,51 @@
+# Check that we can change the VFS mode between 'real' and 'physical', which
+# affects the maximum path length that clang can deal with.
+
+# UNSUPPORTED: system-windows
+
+# RUN: rm -rf xxx*
+# RUN: python %s > %t
+# RUN: cat %t | xargs not %clang 2>&1 | FileCheck %s --check-prefix=PHYSICAL
+# RUN: cat %t | xargs not %clang -ivfsmode=physical  2>&1 | FileCheck %s --check-prefix=PHYSICAL
+# RUN: cat %t | xargs %clang -ivfsmode=real
+
+# PHYSICAL: error: no such file or directory:
+
+import os
+import subprocess
+
+current = os.path.realpath(os.curdir)
+name_max = int(subprocess.check_output("getconf NAME_MAX %s" % current, shell=True))
+path_max = int(subprocess.check_output("getconf PATH_MAX %s" % current, shell=True))
+assert name_max > 0
+assert path_max > 0
+
+filename = 'o.c'
+code = 'int main() { return 0; }'
+
+relative = ''
+absolute = current
+
+# Create directories which almost, but not quite, exhaust PATH_MAX.
+target_len = path_max - 1
+while len(absolute) < target_len:
+new_name_len = min(target_len - len(absolute) - 1, name_max)
+absolute = os.path.join(absolute, 'x' * new_name_len)
+relative = os.path.join(relative, 'x' * new_name_len)
+os.mkdir(relative)
+assert os.path.exists(absolute)
+assert os.path.exists(relative)
+
+# Create a file whose relative path doesn't exceed PATH_MAX, but whose absolute
+# path does.
+file_relative_path = os.path.join(relative, filename)
+with open(file_relative_path, 'w') as f:
+f.write(code)
+
+# At this point only the relative path should work. The absolute one should
+# exceed PATH_MAX.
+assert os.path.exists(file_relative_path)
+assert not os.path.exists(os.path.join(absolute, filename))
+
+# Print out the relative path so lit can pass to clang.
+print file_relative_path
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -131,10 +131,6 @@
   CCCGenericGCCName(""), Saver(Alloc), CheckInputsExist(true),
   GenReproducer(false), SuppressMissingInputWarning(false) {
 
-  // Provide a sane fallback if no VFS is specified.
-  if (!this->VFS)
-this->VFS = llvm::vfs::createPhysicalFileSystem().release();
-
   Name = llvm::sys::path::filename(ClangExecutable);
   Dir = llvm::sys::path::parent_path(ClangExecutable);
   InstalledDir = Dir; // Provide a sensible default installed dir.
@@ -150,6 +146,42 @@
   ResourceDir = GetResourcesPath(ClangExecutable, CLANG_RESOURCE_DIR);
 }
 
+void Driver::ParseVFSMode(ArrayRef Args) {
+  const std::string OptionPrefix =
+  getOpts().getOption(options::OPT_ivfsmode).getPrefixedName();
+  enum class VFSMode { Unknown, Real, Physical } Mode = VFSMode::Unknown;
+  for (const char *ArgPtr : Args) {
+if (!ArgPtr)
+  continue;
+const StringRef Arg = ArgPtr;
+if (!Arg.startswith(OptionPrefix))
+  continue;
+const StringRef Value = Arg.drop_front(OptionPrefix.size());
+VFSMode M = llvm::StringSwitch(Value)
+.Case("real", VFSMode::Real)
+.Case("physical", VFSMode::Physical)
+.Default(VFSMode::Unknown);
+if (M == VFSMode::Unknown) {
+  Diag(diag::err_drv_unsupported_option_argument) << OptionPrefix << Value;
+  break;
+}
+Mode = M;
+  }
+
+  switch (Mode) {
+  case VFSMode::Unknown:
+if (!this->VFS)
+  this->VFS = llvm::vfs::createPhysicalFileSystem().release();
+break;
+  case VFSMode::Physical:
+this->VFS = llvm::vfs::createPhysicalFileSystem().release();
+break;
+  case VFSMode::Real:
+this->VFS = llvm::vfs::getRealFileSystem().get();
+break;
+  }
+}
+
 void Driver::ParseDriverMode(StringRef ProgramName,
  ArrayRef Args) {
   if (ClangNameParts.isEmpty())
@@ -946,6 +978,10 @@
 }
   }
 
+  // We might try accessing the VFS fairly early, so make sure we have the
+  // right one.
+  ParseVFSMode(ArgList.slice(1));
+
   // We look for the driver mode option early, because the mode can affect
   // how other options are parsed.
   ParseDriverMode(ClangExecutable, ArgList.slice(1));
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2007,6 +2007,9 @@
   Flags<[CC1Option]>;

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:172
+  case VFSMode::Unknown:
+if (!this->VFS) {
+  LLVM_FALLTHROUGH;

jfb wrote:
> JDevlieghere wrote:
> > Is this really necessary? 
> the `Driver` ctor takes a `VFS` parameter which some tools set (otherwise it 
> defaults to `nullptr`), so if the command-line `VFS` isn't specified we want 
> to leave whatever was passed in as-is. If the command line parameter is set 
> the we'll obey that instead.
> 
> So yes I think it's the right thing to do.
FWIW, I got confused by the indentation of the case element. Even though it's 
in the if-case it just acts like a label.  While clever, I think for the sake 
of readability it might be worth just repeating `this->VFS = 
llvm::vfs::createPhysicalFileSystem().release();`. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65986



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

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 214296.
jfb marked an inline comment as done.
jfb added a comment.

- Address more comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65986

Files:
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/vfsmode.py

Index: clang/test/Driver/vfsmode.py
===
--- /dev/null
+++ clang/test/Driver/vfsmode.py
@@ -0,0 +1,51 @@
+# Check that we can change the VFS mode between 'real' and 'physical', which
+# affects the maximum path length that clang can deal with.
+
+# UNSUPPORTED: system-windows
+
+# RUN: rm -rf xxx*
+# RUN: python %s > %t
+# RUN: cat %t | xargs not %clang 2>&1 | FileCheck %s --check-prefix=PHYSICAL
+# RUN: cat %t | xargs not %clang -ivfsmode=physical  2>&1 | FileCheck %s --check-prefix=PHYSICAL
+# RUN: cat %t | xargs %clang -ivfsmode=real
+
+# PHYSICAL: error: no such file or directory:
+
+import os
+import subprocess
+
+current = os.path.realpath(os.curdir)
+name_max = int(subprocess.check_output("getconf NAME_MAX %s" % current, shell=True))
+path_max = int(subprocess.check_output("getconf PATH_MAX %s" % current, shell=True))
+assert name_max > 0
+assert path_max > 0
+
+filename = 'o.c'
+code = 'int main() { return 0; }'
+
+relative = ''
+absolute = current
+
+# Create directories which almost, but not quite, exhaust PATH_MAX.
+target_len = path_max - 1
+while len(absolute) < target_len:
+new_name_len = min(target_len - len(absolute) - 1, name_max)
+absolute = os.path.join(absolute, 'x' * new_name_len)
+relative = os.path.join(relative, 'x' * new_name_len)
+os.mkdir(relative)
+assert os.path.exists(absolute)
+assert os.path.exists(relative)
+
+# Create a file whose relative path doesn't exceed PATH_MAX, but whose absolute
+# path does.
+file_relative_path = os.path.join(relative, filename)
+with open(file_relative_path, 'w') as f:
+f.write(code)
+
+# At this point only the relative path should work. The absolute one should
+# exceed PATH_MAX.
+assert os.path.exists(file_relative_path)
+assert not os.path.exists(os.path.join(absolute, filename))
+
+# Print out the relative path so lit can pass to clang.
+print file_relative_path
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -131,10 +131,6 @@
   CCCGenericGCCName(""), Saver(Alloc), CheckInputsExist(true),
   GenReproducer(false), SuppressMissingInputWarning(false) {
 
-  // Provide a sane fallback if no VFS is specified.
-  if (!this->VFS)
-this->VFS = llvm::vfs::createPhysicalFileSystem().release();
-
   Name = llvm::sys::path::filename(ClangExecutable);
   Dir = llvm::sys::path::parent_path(ClangExecutable);
   InstalledDir = Dir; // Provide a sensible default installed dir.
@@ -150,6 +146,42 @@
   ResourceDir = GetResourcesPath(ClangExecutable, CLANG_RESOURCE_DIR);
 }
 
+void Driver::ParseVFSMode(ArrayRef Args) {
+  const std::string OptionPrefix =
+  getOpts().getOption(options::OPT_ivfsmode).getPrefixedName();
+  enum class VFSMode { Unknown, Real, Physical } Mode = VFSMode::Unknown;
+  for (const char *ArgPtr : Args) {
+if (!ArgPtr)
+  continue;
+const StringRef Arg = ArgPtr;
+if (!Arg.startswith(OptionPrefix))
+  continue;
+const StringRef Value = Arg.drop_front(OptionPrefix.size());
+VFSMode M = llvm::StringSwitch(Value)
+.Case("real", VFSMode::Real)
+.Case("physical", VFSMode::Physical)
+.Default(VFSMode::Unknown);
+if (M == VFSMode::Unknown) {
+  Diag(diag::err_drv_unsupported_option_argument) << OptionPrefix << Value;
+  break;
+}
+Mode = M;
+  }
+
+  switch (Mode) {
+  case VFSMode::Unknown:
+if (!this->VFS) {
+  LLVM_FALLTHROUGH;
+case VFSMode::Physical:
+  this->VFS = llvm::vfs::createPhysicalFileSystem().release();
+}
+break;
+  case VFSMode::Real:
+this->VFS = llvm::vfs::getRealFileSystem().get();
+break;
+  }
+}
+
 void Driver::ParseDriverMode(StringRef ProgramName,
  ArrayRef Args) {
   if (ClangNameParts.isEmpty())
@@ -946,6 +978,10 @@
 }
   }
 
+  // We might try accessing the VFS fairly early, so make sure we have the
+  // right one.
+  ParseVFSMode(ArgList.slice(1));
+
   // We look for the driver mode option early, because the mode can affect
   // how other options are parsed.
   ParseDriverMode(ClangExecutable, ArgList.slice(1));
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2007,6 +2007,9 @@
   Flags<[CC1Option]>;
 def ivfsoverlay : 

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

[Jonas Devlieghere via Phabricator via cfe-commits]

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

Two small suggestions for the test, but this change LGTM.




Comment at: clang/test/Driver/vfsmode.py:33
+new_name_len = target_len - len(absolute) - 1
+new_name_len = new_name_len if new_name_len < name_max else name_max
+absolute = os.path.join(absolute, 'x' * new_name_len)

I like the Pythonic approach, but I wonder if `new_name_len = min(target_len - 
len(absolute) - 1, name_max)` wouldn't be easier to read.  



Comment at: clang/test/Driver/vfsmode.py:48
+# exceed PATH_MAX.
+assert os.path.exists(file_relative_path)
+

Should we add an `assert not os.path.exists(absolute)` for good measure?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65986



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

[PATCH] D65993: [NFC][clang] Adding argument based Phase list filtering to getComplicationPhases

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi created this revision.
plotfi added reviewers: compnerd, aaron.ballman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch removes usage of FinalPhase from anywhere outside of the scope where 
it is used to do argument handling.
It also adds argument based trimming of the Phase list pulled out of the 
Types.def table.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65993

Files:
  clang/include/clang/Driver/Types.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/Types.cpp

Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -6,10 +6,14 @@
 //
 //===--===//
 
+#include "clang/Driver/Driver.h"
+#include "clang/Driver/Options.h"
+#include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Types.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/Option/Arg.h"
 #include 
 #include 
 
@@ -293,6 +297,60 @@
   assert(P.size() <= phases::MaxNumberOfPhases && "Too many phases in list");
 }
 
+void types::getCompilationPhases(const clang::driver::Driver ,
+ llvm::opt::DerivedArgList , ID Id,
+ llvm::SmallVectorImpl ) {
+  llvm::SmallVector PhaseList;
+  types::getCompilationPhases(InputType, PL);
+
+  if (Driver->CCCIsCPP() || DAL.getLastArg(options::OPT_E) ||
+  DAL.getLastArg(options::OPT__SLASH_EP) ||
+  DAL.getLastArg(options::OPT_M, options::OPT_MM) ||
+  DAL.getLastArg(options::OPT__SLASH_P)) {
+// Filter to compiler mode. When the compiler is run as a preprocessor then
+// compilation is not an option.
+// -S runs the compiler in Assembly listing mode.
+for (auto Phase : PhaseList)
+  if (Phase <= phases::Preprocess)
+P.push_back(Phase);
+  } else if (DAL.getLastArg(options::OPT__precompile)) {
+// --precompile only runs up to precompilation.
+// This is a clang extension and is not compatible with GCC.
+for (auto Phase : PhaseList)
+  if (Phase <= phases::Precompile)
+P.push_back(Phase);
+  } else if (DAL.getLastArg(options::OPT_fsyntax_only) ||
+ DAL.getLastArg(options::OPT_print_supported_cpus) ||
+ DAL.getLastArg(options::OPT_module_file_info) ||
+ DAL.getLastArg(options::OPT_verify_pch) ||
+ DAL.getLastArg(options::OPT_rewrite_objc) ||
+ DAL.getLastArg(options::OPT_rewrite_legacy_objc) ||
+ DAL.getLastArg(options::OPT__migrate) ||
+ DAL.getLastArg(options::OPT_emit_iterface_stubs) ||
+ DAL.getLastArg(options::OPT__analyze,
+options::OPT__analyze_auto) ||
+ DAL.getLastArg(options::OPT_emit_ast)) {
+// -{fsyntax-only,-analyze,emit-ast} only run up to the compiler.
+for (auto Phase : PhaseList)
+  if (Phase <= phases::Compile)
+P.push_back(Phase);
+  } else if (DAL.getLastArg(options::OPT_S) ||
+ DAL.getLastArg(options::OPT_emit_llvm)) {
+for (auto Phase : PhaseList)
+  if (Phase <= phases::Backend)
+P.push_back(Phase);
+  } else if (DAL.getLastArg(options::OPT_c)) {
+for (auto Phase : PhaseList)
+  if (Phase <= phases::Assemble)
+P.push_back(Phase);
+  } else {
+// Generally means, do every phase until Link.
+for (auto Phase : PhaseList)
+  P.push_back(Phase);
+  }
+  llvm_unreachable("getCompilationPhases(): Reached end of function.")
+}
+
 ID types::lookupCXXTypeForCType(ID Id) {
   switch (Id) {
   default:
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3157,17 +3157,6 @@
 return;
   }
 
-  Arg *FinalPhaseArg;
-  phases::ID FinalPhase = getFinalPhase(Args, );
-
-  if (FinalPhase == phases::Link) {
-if (Args.hasArg(options::OPT_emit_llvm))
-  Diag(clang::diag::err_drv_emit_llvm_link);
-if (IsCLMode() && LTOMode != LTOK_None &&
-!Args.getLastArgValue(options::OPT_fuse_ld_EQ).equals_lower("lld"))
-  Diag(clang::diag::err_drv_lto_without_lld);
-  }
-
   // Reject -Z* at the top level, these options should never have been exposed
   // by gcc.
   if (Arg *A = Args.getLastArg(options::OPT_Z_Joined))
@@ -3220,15 +3209,6 @@
 Args.eraseArg(options::OPT__SLASH_Yc);
 YcArg = nullptr;
   }
-  if (FinalPhase == phases::Preprocess || Args.hasArg(options::OPT__SLASH_Y_)) {
-// If only preprocessing or /Y- is used, all pch handling is disabled.
-// Rather than check for it everywhere, just remove clang-cl pch-related
-// flags here.
-Args.eraseArg(options::OPT__SLASH_Fp);
-Args.eraseArg(options::OPT__SLASH_Yc);
-Args.eraseArg(options::OPT__SLASH_Yu);
-YcArg = YuArg = nullptr;
-  }
 
   

[PATCH] D65969: [clang][NFC] Consolidating usage of "FinalPhase" in Driver::BuildActions

[Puyan Lotfi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368393: [clang][NFC] Consolidating usage of 
FinalPhase in Driver::BuildActions. (authored by zer0, committed by 
).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65969?vs=214222=214293#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65969

Files:
  cfe/trunk/lib/Driver/Driver.cpp

Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -3157,17 +3157,6 @@
 return;
   }
 
-  Arg *FinalPhaseArg;
-  phases::ID FinalPhase = getFinalPhase(Args, );
-
-  if (FinalPhase == phases::Link) {
-if (Args.hasArg(options::OPT_emit_llvm))
-  Diag(clang::diag::err_drv_emit_llvm_link);
-if (IsCLMode() && LTOMode != LTOK_None &&
-!Args.getLastArgValue(options::OPT_fuse_ld_EQ).equals_lower("lld"))
-  Diag(clang::diag::err_drv_lto_without_lld);
-  }
-
   // Reject -Z* at the top level, these options should never have been exposed
   // by gcc.
   if (Arg *A = Args.getLastArg(options::OPT_Z_Joined))
@@ -3220,15 +3209,6 @@
 Args.eraseArg(options::OPT__SLASH_Yc);
 YcArg = nullptr;
   }
-  if (FinalPhase == phases::Preprocess || Args.hasArg(options::OPT__SLASH_Y_)) {
-// If only preprocessing or /Y- is used, all pch handling is disabled.
-// Rather than check for it everywhere, just remove clang-cl pch-related
-// flags here.
-Args.eraseArg(options::OPT__SLASH_Fp);
-Args.eraseArg(options::OPT__SLASH_Yc);
-Args.eraseArg(options::OPT__SLASH_Yu);
-YcArg = YuArg = nullptr;
-  }
 
   // Builder to be used to build offloading actions.
   OffloadingActionBuilder OffloadBuilder(C, Args, Inputs);
@@ -3237,70 +3217,115 @@
   HeaderModulePrecompileJobAction *HeaderModuleAction = nullptr;
   ActionList LinkerInputs;
 
-  unsigned LastPLSize = 0;
-  for (auto  : Inputs) {
-types::ID InputType = I.first;
-const Arg *InputArg = I.second;
-
-llvm::SmallVector PL;
-types::getCompilationPhases(InputType, PL);
-LastPLSize = PL.size();
+  phases::ID FinalPhase;
+  {
+Arg *FinalPhaseArg;
+FinalPhase = getFinalPhase(Args, );
+
+if (FinalPhase == phases::Link) {
+  if (Args.hasArg(options::OPT_emit_llvm))
+Diag(clang::diag::err_drv_emit_llvm_link);
+  if (IsCLMode() && LTOMode != LTOK_None &&
+  !Args.getLastArgValue(options::OPT_fuse_ld_EQ).equals_lower("lld"))
+Diag(clang::diag::err_drv_lto_without_lld);
+}
+
+if (FinalPhase == phases::Preprocess ||
+Args.hasArg(options::OPT__SLASH_Y_)) {
+  // If only preprocessing or /Y- is used, all pch handling is disabled.
+  // Rather than check for it everywhere, just remove clang-cl pch-related
+  // flags here.
+  Args.eraseArg(options::OPT__SLASH_Fp);
+  Args.eraseArg(options::OPT__SLASH_Yc);
+  Args.eraseArg(options::OPT__SLASH_Yu);
+  YcArg = YuArg = nullptr;
+}
+
+unsigned LastPLSize = 0;
+for (auto  : Inputs) {
+  types::ID InputType = I.first;
+  const Arg *InputArg = I.second;
+
+  llvm::SmallVector PL;
+  types::getCompilationPhases(InputType, PL);
+  LastPLSize = PL.size();
+
+  // If the first step comes after the final phase we are doing as part of
+  // this compilation, warn the user about it.
+  phases::ID InitialPhase = PL[0];
+  if (InitialPhase > FinalPhase) {
+if (InputArg->isClaimed())
+  continue;
 
-// If the first step comes after the final phase we are doing as part of
-// this compilation, warn the user about it.
-phases::ID InitialPhase = PL[0];
-if (InitialPhase > FinalPhase) {
-  if (InputArg->isClaimed())
-continue;
+// Claim here to avoid the more general unused warning.
+InputArg->claim();
 
-  // Claim here to avoid the more general unused warning.
-  InputArg->claim();
+// Suppress all unused style warnings with -Qunused-arguments
+if (Args.hasArg(options::OPT_Qunused_arguments))
+  continue;
 
-  // Suppress all unused style warnings with -Qunused-arguments
-  if (Args.hasArg(options::OPT_Qunused_arguments))
+// Special case when final phase determined by binary name, rather than
+// by a command-line argument with a corresponding Arg.
+if (CCCIsCPP())
+  Diag(clang::diag::warn_drv_input_file_unused_by_cpp)
+  << InputArg->getAsString(Args) << getPhaseName(InitialPhase);
+// Special case '-E' warning on a previously preprocessed file to make
+// more sense.
+else if (InitialPhase == phases::Compile &&
+ (Args.getLastArg(options::OPT__SLASH_EP,
+  options::OPT__SLASH_P) ||
+ 

r368393 - [clang][NFC] Consolidating usage of "FinalPhase" in Driver::BuildActions.

[Puyan Lotfi via cfe-commits]

Author: zer0
Date: Thu Aug  8 21:55:09 2019
New Revision: 368393

URL: http://llvm.org/viewvc/llvm-project?rev=368393=rev
Log:
[clang][NFC] Consolidating usage of "FinalPhase" in Driver::BuildActions.

I am working to remove this concept of the "FinalPhase" in the clang driver,
but it is used in a lot of different places to do argument handling for
different combinations of phase pipelines and arguments. I am trying to
consolidate most of the uses of "FinalPhase" into its own separate scope.
Eventually, in a subsequent patch I will move all of this stuff to a separate
function, and have more of the complication phase list construction setup into
types::getComplicationPhases.

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



Modified:
cfe/trunk/lib/Driver/Driver.cpp

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=368393=368392=368393=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Thu Aug  8 21:55:09 2019
@@ -3157,17 +3157,6 @@ void Driver::BuildActions(Compilation 
 return;
   }
 
-  Arg *FinalPhaseArg;
-  phases::ID FinalPhase = getFinalPhase(Args, );
-
-  if (FinalPhase == phases::Link) {
-if (Args.hasArg(options::OPT_emit_llvm))
-  Diag(clang::diag::err_drv_emit_llvm_link);
-if (IsCLMode() && LTOMode != LTOK_None &&
-!Args.getLastArgValue(options::OPT_fuse_ld_EQ).equals_lower("lld"))
-  Diag(clang::diag::err_drv_lto_without_lld);
-  }
-
   // Reject -Z* at the top level, these options should never have been exposed
   // by gcc.
   if (Arg *A = Args.getLastArg(options::OPT_Z_Joined))
@@ -3220,15 +3209,6 @@ void Driver::BuildActions(Compilation 
 Args.eraseArg(options::OPT__SLASH_Yc);
 YcArg = nullptr;
   }
-  if (FinalPhase == phases::Preprocess || Args.hasArg(options::OPT__SLASH_Y_)) 
{
-// If only preprocessing or /Y- is used, all pch handling is disabled.
-// Rather than check for it everywhere, just remove clang-cl pch-related
-// flags here.
-Args.eraseArg(options::OPT__SLASH_Fp);
-Args.eraseArg(options::OPT__SLASH_Yc);
-Args.eraseArg(options::OPT__SLASH_Yu);
-YcArg = YuArg = nullptr;
-  }
 
   // Builder to be used to build offloading actions.
   OffloadingActionBuilder OffloadBuilder(C, Args, Inputs);
@@ -3237,70 +3217,115 @@ void Driver::BuildActions(Compilation 
   HeaderModulePrecompileJobAction *HeaderModuleAction = nullptr;
   ActionList LinkerInputs;
 
-  unsigned LastPLSize = 0;
+  phases::ID FinalPhase;
+  {
+Arg *FinalPhaseArg;
+FinalPhase = getFinalPhase(Args, );
+
+if (FinalPhase == phases::Link) {
+  if (Args.hasArg(options::OPT_emit_llvm))
+Diag(clang::diag::err_drv_emit_llvm_link);
+  if (IsCLMode() && LTOMode != LTOK_None &&
+  !Args.getLastArgValue(options::OPT_fuse_ld_EQ).equals_lower("lld"))
+Diag(clang::diag::err_drv_lto_without_lld);
+}
+
+if (FinalPhase == phases::Preprocess ||
+Args.hasArg(options::OPT__SLASH_Y_)) {
+  // If only preprocessing or /Y- is used, all pch handling is disabled.
+  // Rather than check for it everywhere, just remove clang-cl pch-related
+  // flags here.
+  Args.eraseArg(options::OPT__SLASH_Fp);
+  Args.eraseArg(options::OPT__SLASH_Yc);
+  Args.eraseArg(options::OPT__SLASH_Yu);
+  YcArg = YuArg = nullptr;
+}
+
+unsigned LastPLSize = 0;
+for (auto  : Inputs) {
+  types::ID InputType = I.first;
+  const Arg *InputArg = I.second;
+
+  llvm::SmallVector PL;
+  types::getCompilationPhases(InputType, PL);
+  LastPLSize = PL.size();
+
+  // If the first step comes after the final phase we are doing as part of
+  // this compilation, warn the user about it.
+  phases::ID InitialPhase = PL[0];
+  if (InitialPhase > FinalPhase) {
+if (InputArg->isClaimed())
+  continue;
+
+// Claim here to avoid the more general unused warning.
+InputArg->claim();
+
+// Suppress all unused style warnings with -Qunused-arguments
+if (Args.hasArg(options::OPT_Qunused_arguments))
+  continue;
+
+// Special case when final phase determined by binary name, rather than
+// by a command-line argument with a corresponding Arg.
+if (CCCIsCPP())
+  Diag(clang::diag::warn_drv_input_file_unused_by_cpp)
+  << InputArg->getAsString(Args) << getPhaseName(InitialPhase);
+// Special case '-E' warning on a previously preprocessed file to make
+// more sense.
+else if (InitialPhase == phases::Compile &&
+ (Args.getLastArg(options::OPT__SLASH_EP,
+  options::OPT__SLASH_P) ||
+  Args.getLastArg(options::OPT_E) ||
+  Args.getLastArg(options::OPT_M, options::OPT_MM)) &&
+ 

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 214292.
jfb added a comment.

- Undo whitespaces change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65986

Files:
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/vfsmode.py

Index: clang/test/Driver/vfsmode.py
===
--- /dev/null
+++ clang/test/Driver/vfsmode.py
@@ -0,0 +1,51 @@
+# Check that we can change the VFS mode between 'real' and 'physical', which
+# affects the maximum path length that clang can deal with.
+
+# UNSUPPORTED: system-windows
+
+# RUN: rm -rf xxx*
+# RUN: python %s > %t
+# RUN: cat %t | xargs not %clang 2>&1 | FileCheck %s --check-prefix=PHYSICAL
+# RUN: cat %t | xargs not %clang -ivfsmode=physical  2>&1 | FileCheck %s --check-prefix=PHYSICAL
+# RUN: cat %t | xargs %clang -ivfsmode=real
+
+# PHYSICAL: error: no such file or directory:
+
+import os
+import subprocess
+
+current = os.path.realpath(os.curdir)
+name_max = int(subprocess.check_output("getconf NAME_MAX %s" % current, shell=True))
+path_max = int(subprocess.check_output("getconf PATH_MAX %s" % current, shell=True))
+assert name_max > 0
+assert path_max > 0
+
+filename = 'o.c'
+code = 'int main() { return 0; }'
+
+relative = ''
+absolute = current
+
+# Create directories which almost, but not quite, exhaust PATH_MAX.
+target_len = path_max - 1
+while len(absolute) < target_len:
+new_name_len = target_len - len(absolute) - 1
+new_name_len = new_name_len if new_name_len < name_max else name_max
+absolute = os.path.join(absolute, 'x' * new_name_len)
+relative = os.path.join(relative, 'x' * new_name_len)
+os.mkdir(relative)
+assert os.path.exists(absolute)
+assert os.path.exists(relative)
+
+# Create a file whose relative path doesn't exceed PATH_MAX, but whose absolute
+# path does.
+file_relative_path = os.path.join(relative, filename)
+with open(file_relative_path, 'w') as f:
+f.write(code)
+
+# At this point only the relative path should work. The absolute one should
+# exceed PATH_MAX.
+assert os.path.exists(file_relative_path)
+
+# Print out the relative path so lit can pass to clang.
+print file_relative_path
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -131,10 +131,6 @@
   CCCGenericGCCName(""), Saver(Alloc), CheckInputsExist(true),
   GenReproducer(false), SuppressMissingInputWarning(false) {
 
-  // Provide a sane fallback if no VFS is specified.
-  if (!this->VFS)
-this->VFS = llvm::vfs::createPhysicalFileSystem().release();
-
   Name = llvm::sys::path::filename(ClangExecutable);
   Dir = llvm::sys::path::parent_path(ClangExecutable);
   InstalledDir = Dir; // Provide a sensible default installed dir.
@@ -150,6 +146,42 @@
   ResourceDir = GetResourcesPath(ClangExecutable, CLANG_RESOURCE_DIR);
 }
 
+void Driver::ParseVFSMode(ArrayRef Args) {
+  const std::string OptionPrefix =
+  getOpts().getOption(options::OPT_ivfsmode).getPrefixedName();
+  enum class VFSMode { Unknown, Real, Physical } Mode = VFSMode::Unknown;
+  for (const char *ArgPtr : Args) {
+if (!ArgPtr)
+  continue;
+const StringRef Arg = ArgPtr;
+if (!Arg.startswith(OptionPrefix))
+  continue;
+const StringRef Value = Arg.drop_front(OptionPrefix.size());
+VFSMode M = llvm::StringSwitch(Value)
+.Case("real", VFSMode::Real)
+.Case("physical", VFSMode::Physical)
+.Default(VFSMode::Unknown);
+if (M == VFSMode::Unknown) {
+  Diag(diag::err_drv_unsupported_option_argument) << OptionPrefix << Value;
+  break;
+}
+Mode = M;
+  }
+
+  switch (Mode) {
+  case VFSMode::Unknown:
+if (!this->VFS) {
+  LLVM_FALLTHROUGH;
+case VFSMode::Physical:
+  this->VFS = llvm::vfs::createPhysicalFileSystem().release();
+}
+break;
+  case VFSMode::Real:
+this->VFS = llvm::vfs::getRealFileSystem().get();
+break;
+  }
+}
+
 void Driver::ParseDriverMode(StringRef ProgramName,
  ArrayRef Args) {
   if (ClangNameParts.isEmpty())
@@ -946,6 +978,10 @@
 }
   }
 
+  // We might try accessing the VFS fairly early, so make sure we have the
+  // right one.
+  ParseVFSMode(ArgList.slice(1));
+
   // We look for the driver mode option early, because the mode can affect
   // how other options are parsed.
   ParseDriverMode(ClangExecutable, ArgList.slice(1));
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2007,6 +2007,9 @@
   Flags<[CC1Option]>;
 def ivfsoverlay : JoinedOrSeparate<["-"], "ivfsoverlay">, Group, 

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

[JF Bastien via Phabricator via cfe-commits]

jfb added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:172
+  case VFSMode::Unknown:
+if (!this->VFS) {
+  LLVM_FALLTHROUGH;

JDevlieghere wrote:
> Is this really necessary? 
the `Driver` ctor takes a `VFS` parameter which some tools set (otherwise it 
defaults to `nullptr`), so if the command-line `VFS` isn't specified we want to 
leave whatever was passed in as-is. If the command line parameter is set the 
we'll obey that instead.

So yes I think it's the right thing to do.



Comment at: clang/test/Driver/vfsmode.py:18
+current = os.path.realpath(os.curdir)
+name_max = int(subprocess.check_output("getconf NAME_MAX /", shell=True))
+path_max = int(subprocess.check_output("getconf PATH_MAX /", shell=True))

JDevlieghere wrote:
> Can we pass `current` here?
Nice catch, I think this is required for correctness (in case the CWD's 
filesystem is different from `/`'s and they have different configurations).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65986



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

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 214291.
jfb marked 6 inline comments as done.
jfb added a comment.

- Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65986

Files:
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/vfsmode.py

Index: clang/test/Driver/vfsmode.py
===
--- /dev/null
+++ clang/test/Driver/vfsmode.py
@@ -0,0 +1,51 @@
+# Check that we can change the VFS mode between 'real' and 'physical', which
+# affects the maximum path length that clang can deal with.
+
+# UNSUPPORTED: system-windows
+
+# RUN: rm -rf xxx*
+# RUN: python %s > %t
+# RUN: cat %t | xargs not %clang 2>&1 | FileCheck %s --check-prefix=PHYSICAL
+# RUN: cat %t | xargs not %clang -ivfsmode=physical  2>&1 | FileCheck %s --check-prefix=PHYSICAL
+# RUN: cat %t | xargs %clang -ivfsmode=real
+
+# PHYSICAL: error: no such file or directory:
+
+import os
+import subprocess
+
+current = os.path.realpath(os.curdir)
+name_max = int(subprocess.check_output("getconf NAME_MAX %s" % current, shell=True))
+path_max = int(subprocess.check_output("getconf PATH_MAX %s" % current, shell=True))
+assert name_max > 0
+assert path_max > 0
+
+filename = 'o.c'
+code = 'int main() { return 0; }'
+
+relative = ''
+absolute = current
+
+# Create directories which almost, but not quite, exhaust PATH_MAX.
+target_len = path_max - 1
+while len(absolute) < target_len:
+new_name_len = target_len - len(absolute) - 1
+new_name_len = new_name_len if new_name_len < name_max else name_max
+absolute = os.path.join(absolute, 'x' * new_name_len)
+relative = os.path.join(relative, 'x' * new_name_len)
+os.mkdir(relative)
+assert os.path.exists(absolute)
+assert os.path.exists(relative)
+
+# Create a file whose relative path doesn't exceed PATH_MAX, but whose absolute
+# path does.
+file_relative_path = os.path.join(relative, filename)
+with open(file_relative_path, 'w') as f:
+f.write(code)
+
+# At this point only the relative path should work. The absolute one should
+# exceed PATH_MAX.
+assert os.path.exists(file_relative_path)
+
+# Print out the relative path so lit can pass to clang.
+print file_relative_path
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -131,10 +131,6 @@
   CCCGenericGCCName(""), Saver(Alloc), CheckInputsExist(true),
   GenReproducer(false), SuppressMissingInputWarning(false) {
 
-  // Provide a sane fallback if no VFS is specified.
-  if (!this->VFS)
-this->VFS = llvm::vfs::createPhysicalFileSystem().release();
-
   Name = llvm::sys::path::filename(ClangExecutable);
   Dir = llvm::sys::path::parent_path(ClangExecutable);
   InstalledDir = Dir; // Provide a sensible default installed dir.
@@ -150,6 +146,42 @@
   ResourceDir = GetResourcesPath(ClangExecutable, CLANG_RESOURCE_DIR);
 }
 
+void Driver::ParseVFSMode(ArrayRef Args) {
+  const std::string OptionPrefix =
+  getOpts().getOption(options::OPT_ivfsmode).getPrefixedName();
+  enum class VFSMode { Unknown, Real, Physical } Mode = VFSMode::Unknown;
+  for (const char *ArgPtr : Args) {
+if (!ArgPtr)
+  continue;
+const StringRef Arg = ArgPtr;
+if (!Arg.startswith(OptionPrefix))
+  continue;
+const StringRef Value = Arg.drop_front(OptionPrefix.size());
+VFSMode M = llvm::StringSwitch(Value)
+.Case("real", VFSMode::Real)
+.Case("physical", VFSMode::Physical)
+.Default(VFSMode::Unknown);
+if (M == VFSMode::Unknown) {
+  Diag(diag::err_drv_unsupported_option_argument) << OptionPrefix << Value;
+  break;
+}
+Mode = M;
+  }
+
+  switch (Mode) {
+  case VFSMode::Unknown:
+if (!this->VFS) {
+  LLVM_FALLTHROUGH;
+case VFSMode::Physical:
+  this->VFS = llvm::vfs::createPhysicalFileSystem().release();
+}
+break;
+  case VFSMode::Real:
+this->VFS = llvm::vfs::getRealFileSystem().get();
+break;
+  }
+}
+
 void Driver::ParseDriverMode(StringRef ProgramName,
  ArrayRef Args) {
   if (ClangNameParts.isEmpty())
@@ -946,6 +978,10 @@
 }
   }
 
+  // We might try accessing the VFS fairly early, so make sure we have the
+  // right one.
+  ParseVFSMode(ArgList.slice(1));
+
   // We look for the driver mode option early, because the mode can affect
   // how other options are parsed.
   ParseDriverMode(ClangExecutable, ArgList.slice(1));
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2005,8 +2005,10 @@
   HelpText<"Add directory to SYSTEM include search path, "
   

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

[Csaba Dabis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368383: [analyzer] CastValueChecker: Model castAs(), getAs() 
(authored by Charusso, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65889?vs=214277=214284#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65889

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  cfe/trunk/test/Analysis/cast-value.cpp

Index: cfe/trunk/test/Analysis/cast-value.cpp
===
--- cfe/trunk/test/Analysis/cast-value.cpp
+++ cfe/trunk/test/Analysis/cast-value.cpp
@@ -14,20 +14,33 @@
 const X *cast(Y Value);
 
 template 
-const X *dyn_cast(Y Value);
+const X *dyn_cast(Y *Value);
+template 
+const X _cast(Y );
 
 template 
 const X *cast_or_null(Y Value);
 
 template 
-const X *dyn_cast_or_null(Y Value);
+const X *dyn_cast_or_null(Y *Value);
+template 
+const X *dyn_cast_or_null(Y );
 } // namespace llvm
 
-using namespace llvm;
-
-class Shape {};
+namespace clang {
+struct Shape {
+  template 
+  const T *castAs() const;
+
+  template 
+  const T *getAs() const;
+};
 class Triangle : public Shape {};
 class Circle : public Shape {};
+} // namespace clang
+
+using namespace llvm;
+using namespace clang;
 
 namespace test_cast {
 void evalLogic(const Shape *S) {
@@ -91,8 +104,52 @@
   if (!S)
 clang_analyzer_eval(!C); // logic-warning {{TRUE}}
 }
+} // namespace test_dyn_cast_or_null
 
-void evalNonNullParamNonNullReturn(const Shape *S) {
+namespace test_cast_as {
+void evalLogic(const Shape *S) {
+  const Circle *C = S->castAs();
+  clang_analyzer_numTimesReached(); // logic-warning {{1}}
+
+  if (S && C)
+clang_analyzer_eval(C == S);
+  // logic-warning@-1 {{TRUE}}
+
+  if (S && !C)
+clang_analyzer_warnIfReached(); // no-warning
+
+  if (!S)
+clang_analyzer_warnIfReached(); // no-warning
+}
+} // namespace test_cast_as
+
+namespace test_get_as {
+void evalLogic(const Shape *S) {
+  const Circle *C = S->getAs();
+  clang_analyzer_numTimesReached(); // logic-warning {{2}}
+
+  if (S && C)
+clang_analyzer_eval(C == S);
+  // logic-warning@-1 {{TRUE}}
+
+  if (S && !C)
+clang_analyzer_warnIfReached(); // logic-warning {{REACHABLE}}
+
+  if (!S)
+clang_analyzer_warnIfReached(); // no-warning
+}
+} // namespace test_get_as
+
+namespace test_notes {
+void evalReferences(const Shape ) {
+  const auto  = dyn_cast(S);
+  // expected-note@-1 {{Assuming dynamic cast from 'Shape' to 'Circle' fails}}
+  // expected-note@-2 {{Dereference of null pointer}}
+  // expected-warning@-3 {{Dereference of null pointer}}
+  // logic-warning@-4 {{Dereference of null pointer}}
+}
+
+void evalNonNullParamNonNullReturnReference(const Shape ) {
   const auto *C = dyn_cast_or_null(S);
   // expected-note@-1 {{Assuming dynamic cast from 'Shape' to 'Circle' succeeds}}
   // expected-note@-2 {{Assuming pointer value is null}}
@@ -105,6 +162,19 @@
   // logic-warning@-4 {{Division by zero}}
 }
 
+void evalNonNullParamNonNullReturn(const Shape *S) {
+  const auto *C = cast(S);
+  // expected-note@-1 {{Checked cast from 'Shape' to 'Circle' succeeds}}
+  // expected-note@-2 {{Assuming pointer value is null}}
+  // expected-note@-3 {{'C' initialized here}}
+
+  (void)(1 / !(bool)C);
+  // expected-note@-1 {{'C' is non-null}}
+  // expected-note@-2 {{Division by zero}}
+  // expected-warning@-3 {{Division by zero}}
+  // logic-warning@-4 {{Division by zero}}
+}
+
 void evalNonNullParamNullReturn(const Shape *S) {
   const auto *C = dyn_cast_or_null(S);
   // expected-note@-1 {{Assuming dynamic cast from 'Shape' to 'Circle' fails}}
@@ -134,4 +204,40 @@
   // expected-warning@-2 {{Division by zero}}
   // logic-warning@-3 {{Division by zero}}
 }
-} // namespace test_dyn_cast_or_null
+
+void evalZeroParamNonNullReturnPointer(const Shape *S) {
+  const auto *C = S->castAs();
+  // expected-note@-1 {{Assuming pointer value is null}}
+  // expected-note@-2 {{Checked cast to 'Circle' succeeds}}
+  // expected-note@-3 {{'C' initialized here}}
+
+  (void)(1 / !(bool)C);
+  // expected-note@-1 {{'C' is non-null}}
+  // expected-note@-2 {{Division by zero}}
+  // expected-warning@-3 {{Division by zero}}
+  // logic-warning@-4 {{Division by zero}}
+}
+
+void evalZeroParamNonNullReturn(const Shape ) {
+  const auto *C = S.castAs();
+  // expected-note@-1 {{Checked cast to 'Circle' succeeds}}
+  // expected-note@-2 {{'C' initialized here}}
+
+  (void)(1 / !(bool)C);
+  // expected-note@-1 {{'C' is non-null}}
+  // expected-note@-2 {{Division by zero}}
+  // expected-warning@-3 {{Division by zero}}
+  // logic-warning@-4 {{Division by zero}}
+}
+
+void evalZeroParamNullReturn(const Shape ) {
+  const auto *C = S.getAs();
+  // expected-note@-1 {{Assuming dynamic cast to 'Circle' fails}}
+  // expected-note@-2 {{'C' initialized 

r368383 - [analyzer] CastValueChecker: Model castAs(), getAs()

[Csaba Dabis via cfe-commits]

Author: charusso
Date: Thu Aug  8 19:24:42 2019
New Revision: 368383

URL: http://llvm.org/viewvc/llvm-project?rev=368383=rev
Log:
[analyzer] CastValueChecker: Model castAs(), getAs()

Summary: Thanks to Kristóf Umann for the great idea!

Reviewed By: NoQ

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
cfe/trunk/test/Analysis/cast-value.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp?rev=368383=368382=368383=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp Thu Aug  8 
19:24:42 2019
@@ -16,168 +16,238 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "llvm/ADT/Optional.h"
+#include 
 
 using namespace clang;
 using namespace ento;
 
 namespace {
 class CastValueChecker : public Checker {
+  enum class CastKind { Function, Method };
+
   using CastCheck =
   std::function;
 
+  using CheckKindPair = std::pair;
+
 public:
-  // We have three cases to evaluate a cast:
+  // We have five cases to evaluate a cast:
   // 1) The parameter is non-null, the return value is non-null
   // 2) The parameter is non-null, the return value is null
   // 3) The parameter is null, the return value is null
-  //
   // cast: 1;  dyn_cast: 1, 2;  cast_or_null: 1, 3;  dyn_cast_or_null: 1, 2, 3.
+  //
+  // 4) castAs: has no parameter, the return value is non-null.
+  // 5) getAs:  has no parameter, the return value is null or non-null.
   bool evalCall(const CallEvent , CheckerContext ) const;
 
 private:
-  // These are known in the LLVM project.
-  const CallDescriptionMap CDM = {
-  {{{"llvm", "cast"}, 1}, ::evalCast},
-  {{{"llvm", "dyn_cast"}, 1}, ::evalDynCast},
-  {{{"llvm", "cast_or_null"}, 1}, ::evalCastOrNull},
+  // These are known in the LLVM project. The pairs are in the following form:
+  // {{{namespace, call}, argument-count}, {callback, kind}}
+  const CallDescriptionMap CDM = {
+  {{{"llvm", "cast"}, 1},
+   {::evalCast, CastKind::Function}},
+  {{{"llvm", "dyn_cast"}, 1},
+   {::evalDynCast, CastKind::Function}},
+  {{{"llvm", "cast_or_null"}, 1},
+   {::evalCastOrNull, CastKind::Function}},
   {{{"llvm", "dyn_cast_or_null"}, 1},
-   ::evalDynCastOrNull}};
+   {::evalDynCastOrNull, CastKind::Function}},
+  {{{"clang", "castAs"}, 0},
+   {::evalCastAs, CastKind::Method}},
+  {{{"clang", "getAs"}, 0},
+   {::evalGetAs, CastKind::Method}}};
 
-  void evalCast(const CallExpr *CE, DefinedOrUnknownSVal ParamDV,
+  void evalCast(const CallExpr *CE, DefinedOrUnknownSVal DV,
 CheckerContext ) const;
-  void evalDynCast(const CallExpr *CE, DefinedOrUnknownSVal ParamDV,
+  void evalDynCast(const CallExpr *CE, DefinedOrUnknownSVal DV,
CheckerContext ) const;
-  void evalCastOrNull(const CallExpr *CE, DefinedOrUnknownSVal ParamDV,
+  void evalCastOrNull(const CallExpr *CE, DefinedOrUnknownSVal DV,
   CheckerContext ) const;
-  void evalDynCastOrNull(const CallExpr *CE, DefinedOrUnknownSVal ParamDV,
+  void evalDynCastOrNull(const CallExpr *CE, DefinedOrUnknownSVal DV,
  CheckerContext ) const;
+  void evalCastAs(const CallExpr *CE, DefinedOrUnknownSVal DV,
+  CheckerContext ) const;
+  void evalGetAs(const CallExpr *CE, DefinedOrUnknownSVal DV,
+ CheckerContext ) const;
 };
 } // namespace
 
 static std::string getCastName(const Expr *Cast) {
-  return Cast->getType()->getPointeeCXXRecordDecl()->getNameAsString();
+  QualType Ty = Cast->getType();
+  if (const CXXRecordDecl *RD = Ty->getAsCXXRecordDecl())
+return RD->getNameAsString();
+
+  return Ty->getPointeeCXXRecordDecl()->getNameAsString();
 }
 
-static void evalNonNullParamNonNullReturn(const CallExpr *CE,
-  DefinedOrUnknownSVal ParamDV,
-  CheckerContext ) {
-  ProgramStateRef State = C.getState()->assume(ParamDV, true);
-  if (!State)
-return;
-
-  State = State->BindExpr(CE, C.getLocationContext(), ParamDV, false);
-
-  std::string CastFromName = getCastName(CE->getArg(0));
+static const NoteTag *getCastTag(bool IsNullReturn, const CallExpr *CE,
+ CheckerContext ,
+ bool IsCheckedCast = false) {
+  Optional CastFromName = (CE->getNumArgs() > 0)
+   ? getCastName(CE->getArg(0))
+   : Optional();
   std::string CastToName = getCastName(CE);
 
-  const NoteTag *CastTag = C.getNoteTag(
-  

[PATCH] D65663: [analyzer] ConditionBRVisitor: Fix HTML PathDiagnosticPopUpPieces

[Csaba Dabis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368382: [analyzer] ConditionBRVisitor: Fix HTML 
PathDiagnosticPopUpPieces (authored by Charusso, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65663?vs=213763=214283#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65663

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  cfe/trunk/test/Analysis/Inputs/expected-plists/cxx-for-range.cpp.plist
  cfe/trunk/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
  cfe/trunk/test/Analysis/Inputs/expected-plists/inline-plist.c.plist
  cfe/trunk/test/Analysis/Inputs/expected-plists/objc-radar17039661.m.plist
  cfe/trunk/test/Analysis/Inputs/expected-plists/plist-output.m.plist

Index: cfe/trunk/test/Analysis/Inputs/expected-plists/inline-plist.c.plist
===
--- cfe/trunk/test/Analysis/Inputs/expected-plists/inline-plist.c.plist
+++ cfe/trunk/test/Analysis/Inputs/expected-plists/inline-plist.c.plist
@@ -548,7 +548,7 @@
 
 
  line45
- col12
+ col7
  file0
 

Index: cfe/trunk/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
===
--- cfe/trunk/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
+++ cfe/trunk/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
@@ -2727,7 +2727,7 @@
 
 
  line146
- col13
+ col8
  file0
 

@@ -2949,7 +2949,7 @@
 
 
  line146
- col13
+ col8
  file0
 

@@ -3929,7 +3929,7 @@
 
 
  line178
- col14
+ col9
  file0
 

@@ -4185,7 +4185,7 @@
 
 
  line178
- col14
+ col9
  file0
 

@@ -4281,7 +4281,7 @@
 
 
  line181
- col14
+ col9
  file0
 

@@ -8087,7 +8087,7 @@
  location
  
   line267
-  col18
+  col19
   file0
  
  ranges
@@ -8095,7 +8095,7 @@

 
  line267
- col18
+ col19
  file0
 
 
@@ -8119,12 +8119,12 @@
  
   
line267
-   col18
+   col19
file0
   
   
line267
-   col18
+   col22
file0
   
  
@@ -11983,12 +11983,12 @@
  
   
line457
-   col9
+   col10
file0
   
   
line457
-   col9
+   col14
file0
   
  
@@ -12000,7 +12000,7 @@
  location
  
   line457
-  col9
+  col10
   file0
  
  ranges
@@ -12008,7 +12008,7 @@

 
  line457
- col9
+ col10
  file0
 
 
@@ -12032,12 +12032,12 @@
  
   
line457
-   col9
+   col10
file0
   
   
line457
-   col9
+   col14
file0
   
  
@@ -12244,12 +12244,12 @@
  
   
line457
-   col9
+   col10
file0
   
   
line457
-   col9
+   col14
file0
   
  
@@ -12261,7 +12261,7 @@
  location
  
   line457
-  col9
+  col10
   file0
  
  ranges
@@ -12269,7 +12269,7 @@

 
  line457
- col9
+ col10
  file0
 
 
@@ -12293,12 +12293,12 @@
  
   
line457
-   col9
+   col10
file0
   
   
line457
-   col9
+   col14
file0
   
  
@@ -12571,12 +12571,12 @@
  
   
line457
-   col9
+   col10
file0
   
   
line457
-   col9
+   col14
file0
   
  
@@ -12588,7 +12588,7 @@
  location
  
   line457
-  col9
+  col10
   file0
  
  ranges
@@ -12596,7 +12596,7 @@

 
  line457
- col9
+ col10
  file0
 
 
@@ -12620,12 +12620,12 @@
  
   
line457
-   col9
+   col10
file0
   
   
line457
-   col9
+   col14
file0
   
  
@@ -13128,12 +13128,12 @@
 

r368382 - [analyzer] ConditionBRVisitor: Fix HTML PathDiagnosticPopUpPieces

[Csaba Dabis via cfe-commits]

Author: charusso
Date: Thu Aug  8 19:20:44 2019
New Revision: 368382

URL: http://llvm.org/viewvc/llvm-project?rev=368382=rev
Log:
[analyzer] ConditionBRVisitor: Fix HTML PathDiagnosticPopUpPieces

Summary:
A condition could be a multi-line expression where we create the highlight
in separated chunks. PathDiagnosticPopUpPiece is not made for that purpose,
it cannot be added to multiple lines because we have only one ending part
which contains all the notes. So that it cannot have multiple endings and
therefore this patch narrows down the ranges of the highlight to the given
interesting variable of the condition. It prevents HTML-breaking injections.

Reviewed By: NoQ

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
cfe/trunk/test/Analysis/Inputs/expected-plists/cxx-for-range.cpp.plist
cfe/trunk/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
cfe/trunk/test/Analysis/Inputs/expected-plists/inline-plist.c.plist
cfe/trunk/test/Analysis/Inputs/expected-plists/objc-radar17039661.m.plist
cfe/trunk/test/Analysis/Inputs/expected-plists/plist-output.m.plist

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=368382=368381=368382=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Thu Aug  8 
19:20:44 2019
@@ -2335,12 +2335,12 @@ std::shared_ptr Con
   // Check if the field name of the MemberExprs is ambiguous. Example:
   // " 'a.d' is equal to 'h.d' " in 'test/Analysis/null-deref-path-notes.cpp'.
   bool IsSameFieldName = false;
-  if (const auto *LhsME =
-  dyn_cast(BExpr->getLHS()->IgnoreParenCasts()))
-if (const auto *RhsME =
-dyn_cast(BExpr->getRHS()->IgnoreParenCasts()))
-  IsSameFieldName = LhsME->getMemberDecl()->getName() ==
-RhsME->getMemberDecl()->getName();
+  const auto *LhsME = 
dyn_cast(BExpr->getLHS()->IgnoreParenCasts());
+  const auto *RhsME = 
dyn_cast(BExpr->getRHS()->IgnoreParenCasts());
+
+  if (LhsME && RhsME)
+IsSameFieldName =
+LhsME->getMemberDecl()->getName() == RhsME->getMemberDecl()->getName();
 
   SmallString<128> LhsString, RhsString;
   {
@@ -2410,16 +2410,31 @@ std::shared_ptr Con
 
   Out << (shouldInvert ? LhsString : RhsString);
   const LocationContext *LCtx = N->getLocationContext();
-  PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LCtx);
+  const SourceManager  = BRC.getSourceManager();
 
   // Convert 'field ...' to 'Field ...' if it is a MemberExpr.
   std::string Message = Out.str();
   Message[0] = toupper(Message[0]);
 
-  // If we know the value create a pop-up note.
-  if (!IsAssuming)
+  // If we know the value create a pop-up note to the value part of 'BExpr'.
+  if (!IsAssuming) {
+PathDiagnosticLocation Loc;
+if (!shouldInvert) {
+  if (LhsME && LhsME->getMemberLoc().isValid())
+Loc = PathDiagnosticLocation(LhsME->getMemberLoc(), SM);
+  else
+Loc = PathDiagnosticLocation(BExpr->getLHS(), SM, LCtx);
+} else {
+  if (RhsME && RhsME->getMemberLoc().isValid())
+Loc = PathDiagnosticLocation(RhsME->getMemberLoc(), SM);
+  else
+Loc = PathDiagnosticLocation(BExpr->getRHS(), SM, LCtx);
+}
+
 return std::make_shared(Loc, Message);
+  }
 
+  PathDiagnosticLocation Loc(Cond, SM, LCtx);
   auto event = std::make_shared(Loc, Message);
   if (shouldPrune.hasValue())
 event->setPrunable(shouldPrune.getValue());
@@ -2472,12 +2487,14 @@ std::shared_ptr Con
 return nullptr;
 
   const LocationContext *LCtx = N->getLocationContext();
-  PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LCtx);
 
-  // If we know the value create a pop-up note.
-  if (!IsAssuming)
+  // If we know the value create a pop-up note to the 'DRE'.
+  if (!IsAssuming) {
+PathDiagnosticLocation Loc(DRE, BRC.getSourceManager(), LCtx);
 return std::make_shared(Loc, Out.str());
+  }
 
+  PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LCtx);
   auto event = std::make_shared(Loc, Out.str());
   const ProgramState *state = N->getState().get();
   if (const MemRegion *R = state->getLValue(VD, LCtx).getAsRegion()) {
@@ -2505,11 +2522,17 @@ std::shared_ptr Con
 return nullptr;
 
   const LocationContext *LCtx = N->getLocationContext();
-  PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LCtx);
+  PathDiagnosticLocation Loc;
+
+  // If we know the value create a pop-up note to the member of the MemberExpr.
+  if (!IsAssuming && ME->getMemberLoc().isValid())
+Loc = PathDiagnosticLocation(ME->getMemberLoc(), BRC.getSourceManager());
+  else
+Loc = PathDiagnosticLocation(Cond, 

[PATCH] D65974: Driver: Don't warn about assembler flags being unused when not assembling AND not using the integrated assembler

[Douglas Yung via Phabricator via cfe-commits]

dyung marked an inline comment as done.
dyung added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3559
 // CollectArgsForIntegratedAssembler().
-if (TC.useIntegratedAs()) {
+if (TC.useIntegratedAs() || isa(JA)) {
   Args.ClaimAllArgs(options::OPT_mrelax_all);

thakis wrote:
> dyung wrote:
> > thakis wrote:
> > > We shouldn't claim the OPT_m flags if we don't use integrated-as. 
> > > Claiming Wa_COMMA and Xassembler makes sense to me.
> > If we do not, then one of the tests you included in this change will fail 
> > if a platform defaults to not using the integrated assembler. Should I 
> > remove this change and just update the test to explicitly set 
> > -fintegrated-as/-fno-integrated-as and test for the expected result?
> Yes, tests for the -m flags should pass `-fintegrated-as`. Sorry about 
> missing that. Out of curiosity, which platforms still use external assemblers 
> by default?
Thanks, I have done this and uploaded a new revision with the test changes.

We use an external assembler internally for some legacy projects/reasons.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65974



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

[PATCH] D65974: Driver: Don't warn about assembler flags being unused when not assembling AND not using the integrated assembler

[Douglas Yung via Phabricator via cfe-commits]

dyung updated this revision to Diff 214278.
dyung added a comment.

Reverted changes in Clang.cpp and updated test to expect warning when an 
external assembler is requested.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65974

Files:
  clang/test/Driver/as-options.s


Index: clang/test/Driver/as-options.s
===
--- clang/test/Driver/as-options.s
+++ clang/test/Driver/as-options.s
@@ -39,30 +39,62 @@
 // Test that assembler options don't cause warnings when there's no assembler
 // stage.
 
-// RUN: %clang -mincremental-linker-compatible -E \
+// RUN: %clang -mincremental-linker-compatible -E -fintegrated-as \
 // RUN:   -o /dev/null -x c++ %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
-// RUN: %clang -mincremental-linker-compatible -E \
+// RUN: %clang -mincremental-linker-compatible -E -fno-integrated-as \
+// RUN:   -o /dev/null -x c++ %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN --allow-empty %s
+
+// RUN: %clang -mincremental-linker-compatible -E -fintegrated-as \
 // RUN:   -o /dev/null -x assembler-with-cpp %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
+// RUN: %clang -mincremental-linker-compatible -E -fno-integrated-as \
+// RUN:   -o /dev/null -x assembler-with-cpp %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN --allow-empty %s
+
 // RUN: %clang -mimplicit-it=always -target armv7-linux-gnueabi -E \
-// RUN:   -o /dev/null -x c++ %s 2>&1 \
+// RUN:   -fintegrated-as -o /dev/null -x c++ %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
 // RUN: %clang -mimplicit-it=always -target armv7-linux-gnueabi -E \
-// RUN:   -o /dev/null -x assembler-with-cpp %s 2>&1 \
+// RUN:   -fno-integrated-as -o /dev/null -x c++ %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN --allow-empty %s
+
+// RUN: %clang -mimplicit-it=always -target armv7-linux-gnueabi -E \
+// RUN:   -fintegrated-as -o /dev/null -x assembler-with-cpp %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
+// RUN: %clang -mimplicit-it=always -target armv7-linux-gnueabi -E \
+// RUN:   -fno-integrated-as -o /dev/null -x assembler-with-cpp %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN --allow-empty %s
+
+// RUN: %clang -Wa,-mbig-obj -target i386-pc-windows -E -fintegrated-as \
+// RUN:   -o /dev/null -x c++ %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
-// RUN: %clang -Wa,-mbig-obj -target i386-pc-windows -E \
+// RUN: %clang -Wa,-mbig-obj -target i386-pc-windows -E -fno-integrated-as \
 // RUN:   -o /dev/null -x c++ %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
-// RUN: %clang -Wa,-mbig-obj -target i386-pc-windows -E \
+
+// RUN: %clang -Wa,-mbig-obj -target i386-pc-windows -E -fintegrated-as \
 // RUN:   -o /dev/null -x assembler-with-cpp %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
-// RUN: %clang -Xassembler -mbig-obj -target i386-pc-windows -E \
+// RUN: %clang -Wa,-mbig-obj -target i386-pc-windows -E -fno-integrated-as \
+// RUN:   -o /dev/null -x assembler-with-cpp %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
+
+// RUN: %clang -Xassembler -mbig-obj -target i386-pc-windows -E 
-fintegrated-as \
 // RUN:   -o /dev/null -x c++ %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
 // RUN: %clang -Xassembler -mbig-obj -target i386-pc-windows -E \
+// RUN:   -fno-integrated-as -o /dev/null -x c++ %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
+
+// RUN: %clang -Xassembler -mbig-obj -target i386-pc-windows -E 
-fintegrated-as \
 // RUN:   -o /dev/null -x assembler-with-cpp %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
+// RUN: %clang -Xassembler -mbig-obj -target i386-pc-windows -E \
+// RUN:   -fno-integrated-as -o /dev/null -x assembler-with-cpp %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
+
 // NOWARN-NOT: unused
 
 // Test that unsupported arguments do not cause errors when -fno-integrated-as


Index: clang/test/Driver/as-options.s
===
--- clang/test/Driver/as-options.s
+++ clang/test/Driver/as-options.s
@@ -39,30 +39,62 @@
 // Test that assembler options don't cause warnings when there's no assembler
 // stage.
 
-// RUN: %clang -mincremental-linker-compatible -E \
+// RUN: %clang -mincremental-linker-compatible -E -fintegrated-as \
 // RUN:   -o /dev/null -x c++ %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
-// RUN: %clang -mincremental-linker-compatible -E \
+// RUN: %clang -mincremental-linker-compatible -E -fno-integrated-as \
+// RUN:   -o /dev/null -x c++ %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN --allow-empty %s
+
+// RUN: %clang -mincremental-linker-compatible -E -fintegrated-as \
 // RUN:  

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

Thanks you for the review!


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

https://reviews.llvm.org/D65889



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

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:26
 class CastValueChecker : public Checker {
+  enum class CastKind { Checking, Sugar };
+

NoQ wrote:
> Charusso wrote:
> > NoQ wrote:
> > > Please explain "Checking" and "Sugar". Checking what? Sugar... you mean 
> > > syntactic sugar? For what?
> > I have no idea what devs mean by those names:
> > - `Checking` kind:
> > > The cast<> operator is a “checked cast” operation.
> > > The dyn_cast<> operator is a “checking cast” operation.
> > from 
> > http://llvm.org/docs/ProgrammersManual.html#the-isa-cast-and-dyn-cast-templates
> > 
> > - `Sugar` kind:
> > > Member-template castAs. Look through sugar for the 
> > > underlying instance of .
> > > Member-template getAs'. Look through sugar for an instance 
> > > of .
> > from 
> > https://clang.llvm.org/doxygen/classclang_1_1Type.html#a436b8b08ae7f2404b4712d37986194ce
> > and 
> > https://clang.llvm.org/doxygen/classclang_1_1Type.html#adae68e1f4c85ede2d36da45fbefc48a2
> > 
> > - `isa()` would be `Instance` kind:
> > > The isa<> operator works exactly like the Java “instanceof” operator.
> > from 
> > http://llvm.org/docs/ProgrammersManual.html#the-isa-cast-and-dyn-cast-templates
> > 
> > If you could imagine better names, please let me know. I have tried to use 
> > the definitions.
> `{ Function, Method }`?
These are cool identifiers, thanks!



Comment at: clang/test/Analysis/cast-value.cpp:156-167
+void evalNonNullParamNonNullReturn(const Shape *S) {
+  const auto *C = cast(S);
+  // expected-note@-1 {{Dynamic cast from 'Shape' to 'Circle' succeeds}}
+  // expected-note@-2 {{Assuming pointer value is null}}
+  // expected-note@-3 {{'C' initialized here}}
+
+  (void)(1 / !(bool)C);

NoQ wrote:
> Charusso wrote:
> > NoQ wrote:
> > > Mmm, wait a sec. That's a false positive. `cast<>` doesn't accept null 
> > > pointers. We have `cast_or_null` for this.
> > This `Assuming pointer value is null` note is very random. I believe it is 
> > not made by me and my code is fine, so I have printed a graph:
> > {F9759380}
> > Do you see any problem?
> Whoops, sorry, i didn't notice the `!`. Seems fine.
> 
> Yeah, the note is broken. I have another interesting reproducer for a problem 
> with the same note: https://bugs.llvm.org/show_bug.cgi?id=42938
I will leave that not "Done" as `FIXME: Broken notes.`


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

https://reviews.llvm.org/D65889



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

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 214277.
Charusso marked 6 inline comments as done.
Charusso added a comment.

- Better naming.


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

https://reviews.llvm.org/D65889

Files:
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/test/Analysis/cast-value.cpp

Index: clang/test/Analysis/cast-value.cpp
===
--- clang/test/Analysis/cast-value.cpp
+++ clang/test/Analysis/cast-value.cpp
@@ -14,20 +14,33 @@
 const X *cast(Y Value);
 
 template 
-const X *dyn_cast(Y Value);
+const X *dyn_cast(Y *Value);
+template 
+const X _cast(Y );
 
 template 
 const X *cast_or_null(Y Value);
 
 template 
-const X *dyn_cast_or_null(Y Value);
+const X *dyn_cast_or_null(Y *Value);
+template 
+const X *dyn_cast_or_null(Y );
 } // namespace llvm
 
-using namespace llvm;
+namespace clang {
+struct Shape {
+  template 
+  const T *castAs() const;
 
-class Shape {};
+  template 
+  const T *getAs() const;
+};
 class Triangle : public Shape {};
 class Circle : public Shape {};
+} // namespace clang
+
+using namespace llvm;
+using namespace clang;
 
 namespace test_cast {
 void evalLogic(const Shape *S) {
@@ -91,8 +104,52 @@
   if (!S)
 clang_analyzer_eval(!C); // logic-warning {{TRUE}}
 }
+} // namespace test_dyn_cast_or_null
 
-void evalNonNullParamNonNullReturn(const Shape *S) {
+namespace test_cast_as {
+void evalLogic(const Shape *S) {
+  const Circle *C = S->castAs();
+  clang_analyzer_numTimesReached(); // logic-warning {{1}}
+
+  if (S && C)
+clang_analyzer_eval(C == S);
+  // logic-warning@-1 {{TRUE}}
+
+  if (S && !C)
+clang_analyzer_warnIfReached(); // no-warning
+
+  if (!S)
+clang_analyzer_warnIfReached(); // no-warning
+}
+} // namespace test_cast_as
+
+namespace test_get_as {
+void evalLogic(const Shape *S) {
+  const Circle *C = S->getAs();
+  clang_analyzer_numTimesReached(); // logic-warning {{2}}
+
+  if (S && C)
+clang_analyzer_eval(C == S);
+  // logic-warning@-1 {{TRUE}}
+
+  if (S && !C)
+clang_analyzer_warnIfReached(); // logic-warning {{REACHABLE}}
+
+  if (!S)
+clang_analyzer_warnIfReached(); // no-warning
+}
+} // namespace test_get_as
+
+namespace test_notes {
+void evalReferences(const Shape ) {
+  const auto  = dyn_cast(S);
+  // expected-note@-1 {{Assuming dynamic cast from 'Shape' to 'Circle' fails}}
+  // expected-note@-2 {{Dereference of null pointer}}
+  // expected-warning@-3 {{Dereference of null pointer}}
+  // logic-warning@-4 {{Dereference of null pointer}}
+}
+
+void evalNonNullParamNonNullReturnReference(const Shape ) {
   const auto *C = dyn_cast_or_null(S);
   // expected-note@-1 {{Assuming dynamic cast from 'Shape' to 'Circle' succeeds}}
   // expected-note@-2 {{Assuming pointer value is null}}
@@ -105,6 +162,19 @@
   // logic-warning@-4 {{Division by zero}}
 }
 
+void evalNonNullParamNonNullReturn(const Shape *S) {
+  const auto *C = cast(S);
+  // expected-note@-1 {{Checked cast from 'Shape' to 'Circle' succeeds}}
+  // expected-note@-2 {{Assuming pointer value is null}}
+  // expected-note@-3 {{'C' initialized here}}
+
+  (void)(1 / !(bool)C);
+  // expected-note@-1 {{'C' is non-null}}
+  // expected-note@-2 {{Division by zero}}
+  // expected-warning@-3 {{Division by zero}}
+  // logic-warning@-4 {{Division by zero}}
+}
+
 void evalNonNullParamNullReturn(const Shape *S) {
   const auto *C = dyn_cast_or_null(S);
   // expected-note@-1 {{Assuming dynamic cast from 'Shape' to 'Circle' fails}}
@@ -134,4 +204,40 @@
   // expected-warning@-2 {{Division by zero}}
   // logic-warning@-3 {{Division by zero}}
 }
-} // namespace test_dyn_cast_or_null
+
+void evalZeroParamNonNullReturnPointer(const Shape *S) {
+  const auto *C = S->castAs();
+  // expected-note@-1 {{Assuming pointer value is null}}
+  // expected-note@-2 {{Checked cast to 'Circle' succeeds}}
+  // expected-note@-3 {{'C' initialized here}}
+
+  (void)(1 / !(bool)C);
+  // expected-note@-1 {{'C' is non-null}}
+  // expected-note@-2 {{Division by zero}}
+  // expected-warning@-3 {{Division by zero}}
+  // logic-warning@-4 {{Division by zero}}
+}
+
+void evalZeroParamNonNullReturn(const Shape ) {
+  const auto *C = S.castAs();
+  // expected-note@-1 {{Checked cast to 'Circle' succeeds}}
+  // expected-note@-2 {{'C' initialized here}}
+
+  (void)(1 / !(bool)C);
+  // expected-note@-1 {{'C' is non-null}}
+  // expected-note@-2 {{Division by zero}}
+  // expected-warning@-3 {{Division by zero}}
+  // logic-warning@-4 {{Division by zero}}
+}
+
+void evalZeroParamNullReturn(const Shape ) {
+  const auto *C = S.getAs();
+  // expected-note@-1 {{Assuming dynamic cast to 'Circle' fails}}
+  // expected-note@-2 {{'C' initialized to a null pointer value}}
+
+  (void)(1 / (bool)C);
+  // expected-note@-1 {{Division by zero}}
+  // expected-warning@-2 {{Division by zero}}
+  // logic-warning@-3 {{Division by zero}}
+}
+} // namespace test_notes
Index: 

[PATCH] D65989: [clang-scan-deps] Add minimizer support for C++20 modules.

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese closed this revision.
Bigcheese added a comment.

Fixed and committed as r368381.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65989



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

[PATCH] D65919: [clang-tidy] Add llvm-prefer-register-over-unsigned check and apply it to LLVM

[Daniel Sanders via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp:48-51
+for (const auto *UsingDirective: Context->using_directives())
+  if (UsingDirective->getNominatedNamespace()
+  ->getQualifiedNameAsString() == "llvm")
+Replacement = "Register";

There's something not quite right here but I haven't figured out what yet. One 
of the tests (apply_2()) is failing (which is odd because I ran `ninja 
check-clang-tools` before posting the patch) because the `using namespace llvm` 
in the function scope doesn't show up in the DeclContext for the function. Am I 
looking in the wrong place for it?

More generally, I suspect there's an easier way to omit the `llvm::` than the 
way I'm using on lines 43-52.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65919



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

r368381 - [clang-scan-deps] Add minimizer support for C++20 modules.

[Michael J. Spencer via cfe-commits]

Author: mspencer
Date: Thu Aug  8 19:01:10 2019
New Revision: 368381

URL: http://llvm.org/viewvc/llvm-project?rev=368381=rev
Log:
[clang-scan-deps] Add minimizer support for C++20 modules.

This only adds support to the minimizer, it doesn't actually capture the 
dependencies yet.

Modified:
cfe/trunk/include/clang/Lex/DependencyDirectivesSourceMinimizer.h
cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
cfe/trunk/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp

Modified: cfe/trunk/include/clang/Lex/DependencyDirectivesSourceMinimizer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/DependencyDirectivesSourceMinimizer.h?rev=368381=368380=368381=diff
==
--- cfe/trunk/include/clang/Lex/DependencyDirectivesSourceMinimizer.h (original)
+++ cfe/trunk/include/clang/Lex/DependencyDirectivesSourceMinimizer.h Thu Aug  
8 19:01:10 2019
@@ -47,6 +47,9 @@ enum TokenKind {
   pp_else,
   pp_endif,
   decl_at_import,
+  cxx_export_decl,
+  cxx_module_decl,
+  cxx_import_decl,
   pp_eof,
 };
 

Modified: cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp?rev=368381=368380=368381=diff
==
--- cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp (original)
+++ cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp Thu Aug  8 
19:01:10 2019
@@ -59,6 +59,7 @@ private:
   LLVM_NODISCARD bool minimizeImpl(const char *First, const char *const End);
   LLVM_NODISCARD bool lexPPLine(const char *, const char *const End);
   LLVM_NODISCARD bool lexAt(const char *, const char *const End);
+  LLVM_NODISCARD bool lexModule(const char *, const char *const End);
   LLVM_NODISCARD bool lexDefine(const char *, const char *const End);
   LLVM_NODISCARD bool lexPragma(const char *, const char *const End);
   LLVM_NODISCARD bool lexEndif(const char *, const char *const End);
@@ -576,6 +577,59 @@ bool Minimizer::lexAt(const char *
   return false;
 }
 
+bool Minimizer::lexModule(const char *, const char *const End) {
+  IdInfo Id = lexIdentifier(First, End);
+  First = Id.Last;
+  bool Export = false;
+  if (Id.Name == "export") {
+Export = true;
+skipWhitespace(First, End);
+if (!isIdentifierBody(*First)) {
+  skipLine(First, End);
+  return false;
+}
+Id = lexIdentifier(First, End);
+First = Id.Last;
+  }
+
+  if (Id.Name != "module" && Id.Name != "import") {
+skipLine(First, End);
+return false;
+  }
+
+  skipWhitespace(First, End);
+
+  // Ignore this as a module directive if the next character can't be part of
+  // an import.
+
+  switch (*First) {
+  case ':':
+  case '<':
+  case '"':
+break;
+  default:
+if (!isIdentifierBody(*First)) {
+  skipLine(First, End);
+  return false;
+}
+  }
+
+  if (Export) {
+makeToken(cxx_export_decl);
+append("export ");
+  }
+
+  if (Id.Name == "module")
+makeToken(cxx_module_decl);
+  else
+makeToken(cxx_import_decl);
+  append(Id.Name);
+  append(" ");
+  printToNewline(First, End);
+  append("\n");
+  return false;
+}
+
 bool Minimizer::lexDefine(const char *, const char *const End) {
   makeToken(pp_define);
   append("#define ");
@@ -677,6 +731,18 @@ bool Minimizer::lexDefault(TokenKind Kin
   return false;
 }
 
+static bool isStartOfRelevantLine(char First) {
+  switch (First) {
+  case '#':
+  case '@':
+  case 'i':
+  case 'e':
+  case 'm':
+return true;
+  }
+  return false;
+}
+
 bool Minimizer::lexPPLine(const char *, const char *const End) {
   assert(First != End);
 
@@ -685,7 +751,7 @@ bool Minimizer::lexPPLine(const char *
   if (First == End)
 return false;
 
-  if (*First != '#' && *First != '@') {
+  if (!isStartOfRelevantLine(*First)) {
 skipLine(First, End);
 assert(First <= End);
 return false;
@@ -695,6 +761,9 @@ bool Minimizer::lexPPLine(const char *
   if (*First == '@')
 return lexAt(First, End);
 
+  if (*First == 'i' || *First == 'e' || *First == 'm')
+return lexModule(First, End);
+
   // Handle preprocessing directives.
   ++First; // Skip over '#'.
   skipWhitespace(First, End);

Modified: cfe/trunk/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp?rev=368381=368380=368381=diff
==
--- cfe/trunk/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp 
(original)
+++ cfe/trunk/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp Thu Aug 
 8 19:01:10 2019
@@ -60,7 +60,9 @@ TEST(MinimizeSourceToDependencyDirective
"#__include_macros \n"
"#import \n"
  

[PATCH] D65989: [clang-scan-deps] Add minimizer support for C++20 modules.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp:577
+TEST(MinimizeSourceToDependencyDirectivesTest, CxxModules) {
+SmallVector Out;
+SmallVector Tokens;

Looks like it's not indented according to clang-format rules


Repository:
  rC Clang

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

https://reviews.llvm.org/D65989



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

[PATCH] D65919: [clang-tidy] Add llvm-prefer-register-over-unsigned check and apply it to LLVM

[Daniel Sanders via Phabricator via cfe-commits]

dsanders updated this revision to Diff 214276.
dsanders added a comment.

Changed diagnostic message and merged the fixit into the original diagnostic
Made the variable names more distinct in the tests
Added a test to cover `using namespace llvm` in the global namespace
Slight correction to the iteration over the contexts to avoid skipping one
clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65919

Files:
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp
  clang-tools-extra/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-register-over-unsigned.rst
  clang-tools-extra/test/clang-tidy/llvm-prefer-register-over-unsigned.cpp
  clang-tools-extra/test/clang-tidy/llvm-prefer-register-over-unsigned2.cpp

Index: clang-tools-extra/test/clang-tidy/llvm-prefer-register-over-unsigned2.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-prefer-register-over-unsigned2.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s llvm-prefer-register-over-unsigned %t
+
+namespace llvm {
+class Register {
+public:
+  operator unsigned();
+};
+} // end namespace llvm
+
+llvm::Register getReg();
+
+using namespace llvm;
+
+void apply_1() {
+  unsigned Reg = getReg();
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'Reg' declared as 'unsigned int'; use 'Register' instead [llvm-prefer-register-over-unsigned]
+  // CHECK-FIXES: apply_1()
+  // CHECK-FIXES-NEXT: Register Reg = getReg();
+}
+
+void done_1() {
+  llvm::Register Reg = getReg();
+  // CHECK-FIXES: done_1()
+  // CHECK-FIXES-NEXT: llvm::Register Reg = getReg();
+}
Index: clang-tools-extra/test/clang-tidy/llvm-prefer-register-over-unsigned.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-prefer-register-over-unsigned.cpp
@@ -0,0 +1,57 @@
+// RUN: %check_clang_tidy %s llvm-prefer-register-over-unsigned %t
+
+namespace llvm {
+class Register {
+public:
+  operator unsigned();
+};
+} // end namespace llvm
+
+llvm::Register getReg();
+
+void apply_1() {
+  unsigned Reg1 = getReg();
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'Reg1' declared as 'unsigned int'; use 'llvm::Register' instead [llvm-prefer-register-over-unsigned]
+  // CHECK-FIXES: apply_1()
+  // CHECK-FIXES-NEXT: llvm::Register Reg1 = getReg();
+}
+
+void apply_2() {
+  using namespace llvm;
+  unsigned Reg2 = getReg();
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'Reg2' declared as 'unsigned int'; use 'Register' instead [llvm-prefer-register-over-unsigned]
+  // CHECK-FIXES: apply_2()
+  // CHECK-FIXES-NEXT: using namespace llvm;
+  // CHECK-FIXES-NEXT: Register Reg2 = getReg();
+}
+
+namespace llvm {
+void apply_3() {
+  unsigned Reg3 = getReg();
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'Reg3' declared as 'unsigned int'; use 'Register' instead [llvm-prefer-register-over-unsigned]
+  // CHECK-FIXES: apply_3()
+  // CHECK-FIXES-NEXT: Register Reg3 = getReg();
+}
+} // end namespace llvm
+
+void done_1() {
+  llvm::Register Reg1 = getReg();
+  // CHECK-FIXES: done_1()
+  // CHECK-FIXES-NEXT: llvm::Register Reg1 = getReg();
+}
+
+void done_2() {
+  using namespace llvm;
+  Register Reg2 = getReg();
+  // CHECK-FIXES: done_2()
+  // CHECK-FIXES-NEXT: using namespace llvm;
+  // CHECK-FIXES-NEXT: Register Reg2 = getReg();
+}
+
+namespace llvm {
+void done_3() {
+  Register Reg3 = getReg();
+  // CHECK-FIXES: done_3()
+  // CHECK-FIXES-NEXT: Register Reg3 = getReg();
+}
+} // end namespace llvm
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-register-over-unsigned.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-register-over-unsigned.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - llvm-prefer-register-over-unsigned
+
+llvm-prefer-register-over-unsigned
+==
+
+Finds historical use of ``unsigned`` to hold vregs and physregs and rewrites
+them to use ``Register``.
+
+Currently this works by finding all variables of unsigned integer type whose
+initialize begins with an implicit cast from ``Register`` to ``unsigned``.
+
+.. code-block:: c++
+
+  void example(MachineOperand ) {
+unsigned Reg = MO.getReg();
+...
+  }
+
+becomes:
+
+.. code-block:: c++
+
+  void example(MachineOperand ) {
+Register Reg = MO.getReg();
+...
+  }
+
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ 

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

[Charles Zhang via Phabricator via cfe-commits]

czhang updated this revision to Diff 214275.
czhang added a comment.

Fix test and formatting.


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

https://reviews.llvm.org/D62829

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-dynamic-static-initializers.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/bugprone-dynamic-static-initializers.hpp

Index: clang-tools-extra/test/clang-tidy/bugprone-dynamic-static-initializers.hpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/bugprone-dynamic-static-initializers.hpp
@@ -0,0 +1,44 @@
+// RUN: %check_clang_tidy %s bugprone-dynamic-static-initializers %t -- -- -fno-threadsafe-statics
+
+int fact(int n) {
+  return (n == 0) ? 1 : n * fact(n - 1);
+}
+
+int static_thing = fact(5);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: static variable 'static_thing' may be dynamically initialized in this header file [bugprone-dynamic-static-initializers]
+
+int sample() {
+int x;
+return x;
+}
+
+int dynamic_thing = sample();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: static variable 'dynamic_thing' may be dynamically initialized in this header file [bugprone-dynamic-static-initializers]
+
+int not_so_bad = 12 + 4942; // no warning
+
+extern int bar();
+
+int foo() {
+  static int k = bar();
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: static variable 'k' may be dynamically initialized in this header file [bugprone-dynamic-static-initializers]
+  return k;
+}
+
+int bar2() {
+  return 7;
+}
+
+int foo2() {
+  // This may work fine when optimization is enabled because bar() can
+  // be turned into a constant 7.  But without optimization, it can
+  // cause problems. Therefore, we must err on the side of conservatism.
+  static int x = bar2();
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: static variable 'x' may be dynamically initialized in this header file [bugprone-dynamic-static-initializers]
+  return x;
+}
+
+int foo3() {
+  static int p = 7 + 83; // no warning
+  return p;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -43,6 +43,7 @@
bugprone-branch-clone
bugprone-copy-constructor-init
bugprone-dangling-handle
+   bugprone-dynamic-static-initializers
bugprone-exception-escape
bugprone-fold-init-type
bugprone-forward-declaration-namespace
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-dynamic-static-initializers.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-dynamic-static-initializers.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - bugprone-dynamic-static-initializers
+
+bugprone-dynamic-static-initializers
+
+
+Finds instances of static variables that are dynamically initialized
+in header files.
+
+This can pose problems in certain multithreaded contexts. For example,
+when disabling compiler generated synchronization instructions for
+static variables initialized at runtime (e.g. by ``-fno-threadsafe-statics``), even if a particular project
+takes the necessary precautions to prevent race conditions during
+initialization by providing their own synchronization, header files included from other projects may
+not. Therefore, such a check is helpful for ensuring that disabling
+compiler generated synchronization for static variable initialization will not cause
+problems.
+
+Consider the following code:
+
+-- code-block:: c
+
+int foo() {
+  static int k = bar();
+  return k;
+}
+
+When synchronization of static initialization is disabled, if two threads both call `foo` for the first time, there is the possibility that `k` will be double initialized, creating a race condition.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -111,6 +111,12 @@
 
   This checks ensures that ``pipe2()`` is called with the O_CLOEXEC flag.
 
+- New :doc:`bugprone-dynamic-static-initializers
+  ` check.
+
+  Finds instances where variables with static storage are initialized
+  dynamically in header files.
+
 - New :doc:`bugprone-unhandled-self-assignment
   ` check.
 
Index: clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h
===
--- /dev/null
+++ 

[PATCH] D65989: [clang-scan-deps] Add minimizer support for C++20 modules.

[Alex Lorenz via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D65989



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

[PATCH] D65974: Driver: Don't warn about assembler flags being unused when not assembling AND not using the integrated assembler

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3559
 // CollectArgsForIntegratedAssembler().
-if (TC.useIntegratedAs()) {
+if (TC.useIntegratedAs() || isa(JA)) {
   Args.ClaimAllArgs(options::OPT_mrelax_all);

dyung wrote:
> thakis wrote:
> > We shouldn't claim the OPT_m flags if we don't use integrated-as. Claiming 
> > Wa_COMMA and Xassembler makes sense to me.
> If we do not, then one of the tests you included in this change will fail if 
> a platform defaults to not using the integrated assembler. Should I remove 
> this change and just update the test to explicitly set 
> -fintegrated-as/-fno-integrated-as and test for the expected result?
Yes, tests for the -m flags should pass `-fintegrated-as`. Sorry about missing 
that. Out of curiosity, which platforms still use external assemblers by 
default?


Repository:
  rC Clang

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

https://reviews.llvm.org/D65974



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

[PATCH] D65989: [clang-scan-deps] Add minimizer support for C++20 modules.

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese created this revision.
Bigcheese added a reviewer: arphaman.
Herald added subscribers: tschuett, dexonsmith.
Herald added a project: clang.

This only adds support to the minimizer, it doesn't actually capture the 
dependencies yet.


Repository:
  rC Clang

https://reviews.llvm.org/D65989

Files:
  include/clang/Lex/DependencyDirectivesSourceMinimizer.h
  lib/Lex/DependencyDirectivesSourceMinimizer.cpp
  unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp

Index: unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
===
--- unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
+++ unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
@@ -60,7 +60,9 @@
"#__include_macros \n"
"#import \n"
"@import A;\n"
-   "#pragma clang module import A\n",
+   "#pragma clang module import A\n"
+   "export module m;\n"
+   "import m;\n",
Out, Tokens));
   EXPECT_EQ(pp_define, Tokens[0].K);
   EXPECT_EQ(pp_undef, Tokens[1].K);
@@ -76,7 +78,10 @@
   EXPECT_EQ(pp_import, Tokens[11].K);
   EXPECT_EQ(decl_at_import, Tokens[12].K);
   EXPECT_EQ(pp_pragma_import, Tokens[13].K);
-  EXPECT_EQ(pp_eof, Tokens[14].K);
+  EXPECT_EQ(cxx_export_decl, Tokens[14].K);
+  EXPECT_EQ(cxx_module_decl, Tokens[15].K);
+  EXPECT_EQ(cxx_import_decl, Tokens[16].K);
+  EXPECT_EQ(pp_eof, Tokens[17].K);
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, Define) {
@@ -568,4 +573,48 @@
   EXPECT_STREQ("#pragma once\n#include \n", Out.data());
 }
 
+TEST(MinimizeSourceToDependencyDirectivesTest, CxxModules) {
+SmallVector Out;
+SmallVector Tokens;
+
+StringRef Source = R"(
+  module;
+  #include "textual-header.h"
+
+  export module m;
+  exp\
+ort \
+import \
+:l [[rename]];
+
+  export void f();
+
+  void h() {
+import.a = 3;
+import = 3;
+import <<= 3;
+import->a = 3;
+import();
+import . a();
+
+import a b d e d e f e;
+import foo [[no_unique_address]];
+import foo();
+import f(:sefse);
+import f(->a = 3);
+  }
+  )";
+ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out, Tokens));
+EXPECT_STREQ("#include \"textual-header.h\"\nexport module m;\n"
+ "export import :l [[rename]];\n"
+ "import <<= 3;\nimport a b d e d e f e;\n"
+ "import foo [[no_unique_address]];\nimport foo();\n"
+ "import f(:sefse);\nimport f(->a = 3);\n", Out.data());
+ASSERT_EQ(Tokens.size(), 12u);
+EXPECT_EQ(Tokens[0].K,
+  minimize_source_to_dependency_directives::pp_include);
+EXPECT_EQ(Tokens[2].K,
+  minimize_source_to_dependency_directives::cxx_module_decl);
+}
+
 } // end anonymous namespace
Index: lib/Lex/DependencyDirectivesSourceMinimizer.cpp
===
--- lib/Lex/DependencyDirectivesSourceMinimizer.cpp
+++ lib/Lex/DependencyDirectivesSourceMinimizer.cpp
@@ -59,6 +59,7 @@
   LLVM_NODISCARD bool minimizeImpl(const char *First, const char *const End);
   LLVM_NODISCARD bool lexPPLine(const char *, const char *const End);
   LLVM_NODISCARD bool lexAt(const char *, const char *const End);
+  LLVM_NODISCARD bool lexModule(const char *, const char *const End);
   LLVM_NODISCARD bool lexDefine(const char *, const char *const End);
   LLVM_NODISCARD bool lexPragma(const char *, const char *const End);
   LLVM_NODISCARD bool lexEndif(const char *, const char *const End);
@@ -576,6 +577,59 @@
   return false;
 }
 
+bool Minimizer::lexModule(const char *, const char *const End) {
+  IdInfo Id = lexIdentifier(First, End);
+  First = Id.Last;
+  bool Export = false;
+  if (Id.Name == "export") {
+Export = true;
+skipWhitespace(First, End);
+if (!isIdentifierBody(*First)) {
+  skipLine(First, End);
+  return false;
+}
+Id = lexIdentifier(First, End);
+First = Id.Last;
+  }
+
+  if (Id.Name != "module" && Id.Name != "import") {
+skipLine(First, End);
+return false;
+  }
+
+  skipWhitespace(First, End);
+
+  // Ignore this as a module directive if the next character can't be part of
+  // an import.
+
+  switch (*First) {
+  case ':':
+  case '<':
+  case '"':
+break;
+  default:
+if (!isIdentifierBody(*First)) {
+  skipLine(First, End);
+  return false;
+}
+  }
+
+  if (Export) {
+makeToken(cxx_export_decl);
+append("export ");
+  }
+
+  if (Id.Name == "module")
+makeToken(cxx_module_decl);
+  else
+makeToken(cxx_import_decl);
+  append(Id.Name);
+  append(" ");
+  

[PATCH] D65906: [clang-scan-deps] Fix edge cases in the minimizer

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

@aganea These are not just any invisible characters that you have, this is the 
UTF8 BOM. Clang's Lexer skips over them if they're in the beginning of the file 
(`Lexer::InitLexer`). The minimizer should do the same thing, so ideally you 
would factor out the BOM detection and teach the minimizer to skip past it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65906



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

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore marked 3 inline comments as done.
stephanemoore added inline comments.



Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:820
+  llvm::DenseMap>
+  CompatibleAliases;

aaron.ballman wrote:
> gribozavr wrote:
> > `unordered_set`?
> or `SmallPtrSet`?
I went with `llvm::SmallPtrSet` as that seems more common in clang sources 
compared to `std::unordered_set`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60543



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

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

[Artem Dergachev via Phabricator via cfe-commits]

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

Aha, ok!




Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:26
 class CastValueChecker : public Checker {
+  enum class CastKind { Checking, Sugar };
+

Charusso wrote:
> NoQ wrote:
> > Please explain "Checking" and "Sugar". Checking what? Sugar... you mean 
> > syntactic sugar? For what?
> I have no idea what devs mean by those names:
> - `Checking` kind:
> > The cast<> operator is a “checked cast” operation.
> > The dyn_cast<> operator is a “checking cast” operation.
> from 
> http://llvm.org/docs/ProgrammersManual.html#the-isa-cast-and-dyn-cast-templates
> 
> - `Sugar` kind:
> > Member-template castAs. Look through sugar for the 
> > underlying instance of .
> > Member-template getAs'. Look through sugar for an instance 
> > of .
> from 
> https://clang.llvm.org/doxygen/classclang_1_1Type.html#a436b8b08ae7f2404b4712d37986194ce
> and 
> https://clang.llvm.org/doxygen/classclang_1_1Type.html#adae68e1f4c85ede2d36da45fbefc48a2
> 
> - `isa()` would be `Instance` kind:
> > The isa<> operator works exactly like the Java “instanceof” operator.
> from 
> http://llvm.org/docs/ProgrammersManual.html#the-isa-cast-and-dyn-cast-templates
> 
> If you could imagine better names, please let me know. I have tried to use 
> the definitions.
`{ Function, Method }`?



Comment at: clang/test/Analysis/cast-value.cpp:156-167
+void evalNonNullParamNonNullReturn(const Shape *S) {
+  const auto *C = cast(S);
+  // expected-note@-1 {{Dynamic cast from 'Shape' to 'Circle' succeeds}}
+  // expected-note@-2 {{Assuming pointer value is null}}
+  // expected-note@-3 {{'C' initialized here}}
+
+  (void)(1 / !(bool)C);

Charusso wrote:
> NoQ wrote:
> > Mmm, wait a sec. That's a false positive. `cast<>` doesn't accept null 
> > pointers. We have `cast_or_null` for this.
> This `Assuming pointer value is null` note is very random. I believe it is 
> not made by me and my code is fine, so I have printed a graph:
> {F9759380}
> Do you see any problem?
Whoops, sorry, i didn't notice the `!`. Seems fine.

Yeah, the note is broken. I have another interesting reproducer for a problem 
with the same note: https://bugs.llvm.org/show_bug.cgi?id=42938


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

https://reviews.llvm.org/D65889



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

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 214272.
stephanemoore added a comment.

Use `llvm::SmallPtrSet` to store the compatible aliases instead of `std::set`.
Fix a stray unit test failure in `RegistryTest.cpp`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60543

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp

Index: clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
===
--- clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
+++ clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
@@ -439,7 +439,7 @@
   Error.get()).isNull());
   EXPECT_EQ("Incorrect type for arg 1. "
 "(Expected = Matcher) != "
-"(Actual = Matcher"
+"(Actual = Matcher"
 ")",
 Error->toString());
 }
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -573,6 +573,111 @@
   EXPECT_TRUE(notMatches(Code, cxxRecordDecl(isSameOrDerivedFrom("";
 }
 
+TEST(DeclarationMatcher, ObjCClassIsDerived) {
+  DeclarationMatcher IsDerivedFromX = objcInterfaceDecl(isDerivedFrom("X"));
+  EXPECT_TRUE(
+  matchesObjC("@interface X @end @interface Y : X @end", IsDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface X @end @interface Y<__covariant ObjectType> : X @end",
+  IsDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface X @end @compatibility_alias Y X; @interface Z : Y @end",
+  IsDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface X @end typedef X Y; @interface Z : Y @end", IsDerivedFromX));
+  EXPECT_TRUE(notMatchesObjC("@interface X @end", IsDerivedFromX));
+  EXPECT_TRUE(notMatchesObjC("@class X;", IsDerivedFromX));
+  EXPECT_TRUE(notMatchesObjC("@class Y;", IsDerivedFromX));
+  EXPECT_TRUE(notMatchesObjC("@interface X @end @compatibility_alias Y X;",
+ IsDerivedFromX));
+  EXPECT_TRUE(notMatchesObjC("@interface X @end typedef X Y;", IsDerivedFromX));
+
+  DeclarationMatcher IsDirectlyDerivedFromX =
+  objcInterfaceDecl(isDirectlyDerivedFrom("X"));
+  EXPECT_TRUE(
+  matchesObjC("@interface X @end @interface Y : X @end", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface X @end @interface Y<__covariant ObjectType> : X @end",
+  IsDirectlyDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface X @end @compatibility_alias Y X; @interface Z : Y @end",
+  IsDirectlyDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface X @end typedef X Y; @interface Z : Y @end",
+  IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatchesObjC("@interface X @end", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatchesObjC("@class X;", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatchesObjC("@class Y;", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatchesObjC("@interface X @end @compatibility_alias Y X;",
+ IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatchesObjC("@interface X @end typedef X Y;",
+ IsDirectlyDerivedFromX));
+
+  DeclarationMatcher IsAX = objcInterfaceDecl(isSameOrDerivedFrom("X"));
+  EXPECT_TRUE(matchesObjC("@interface X @end @interface Y : X @end", IsAX));
+  EXPECT_TRUE(matchesObjC("@interface X @end", IsAX));
+  EXPECT_TRUE(matchesObjC("@class X;", IsAX));
+  EXPECT_TRUE(notMatchesObjC("@interface Y @end", IsAX));
+  EXPECT_TRUE(notMatchesObjC("@class Y;", IsAX));
+
+  DeclarationMatcher ZIsDerivedFromX =
+  objcInterfaceDecl(hasName("Z"), isDerivedFrom("X"));
+  DeclarationMatcher ZIsDirectlyDerivedFromX =
+  objcInterfaceDecl(hasName("Z"), isDirectlyDerivedFrom("X"));
+  EXPECT_TRUE(matchesObjC(
+  "@interface X @end @interface Y : X @end @interface Z : Y @end",
+  ZIsDerivedFromX));
+  EXPECT_TRUE(matchesObjC("@interface X @end @interface Y : X @end typedef Y "
+  "V; typedef V W; @interface Z : W @end",
+  ZIsDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface X @end typedef X Y; @interface Z : Y @end", ZIsDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface X @end typedef X Y; @interface Z : Y @end",
+  ZIsDirectlyDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface A @end typedef A X; typedef A Y; @interface Z : Y @end",
+  ZIsDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface A @end typedef A X; typedef A Y; @interface Z : Y @end",
+  ZIsDirectlyDerivedFromX));
+  

[PATCH] D65988: [Clang Tablegen] Fix build when std::string::end() is not assignable

[Orivej Desh via Phabricator via cfe-commits]

orivej created this revision.
orivej added a reviewer: clang.
orivej added a project: clang.
Herald added subscribers: cfe-commits, kadircet, arphaman.

This fixes Clang error: expression is not assignable:

  Result.erase(--Result.end()); 
   
   ^    
   


Repository:
  rC Clang

https://reviews.llvm.org/D65988

Files:
  utils/TableGen/ClangDiagnosticsEmitter.cpp


Index: utils/TableGen/ClangDiagnosticsEmitter.cpp
===
--- utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -950,7 +950,7 @@
 Result += '|';
   }
   if (!P->Options.empty())
-Result.erase(--Result.end());
+Result.pop_back();
   Result += '}';
 }
 addInt(mapIndex(P->Index));
@@ -966,7 +966,7 @@
   Result += "|";
 }
 if (!P->Options.empty())
-  Result.erase(--Result.end());
+  Result.pop_back();
 Result += '}';
 addInt(mapIndex(P->Index));
   }


Index: utils/TableGen/ClangDiagnosticsEmitter.cpp
===
--- utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -950,7 +950,7 @@
 Result += '|';
   }
   if (!P->Options.empty())
-Result.erase(--Result.end());
+Result.pop_back();
   Result += '}';
 }
 addInt(mapIndex(P->Index));
@@ -966,7 +966,7 @@
   Result += "|";
 }
 if (!P->Options.empty())
-  Result.erase(--Result.end());
+  Result.pop_back();
 Result += '}';
 addInt(mapIndex(P->Index));
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65987: [clang-doc] Generate HTML links for children namespaces/records

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran created this revision.
DiegoAstiazaran added reviewers: juliehockett, jakehehrlich.
DiegoAstiazaran added a project: clang-tools-extra.

Path is now stored in the references to the child while serializing, then this 
path is used to generate the relative path in the HTML generator.
Now some references have paths and some don't so in the reducing phase, 
references are now properly merged checking for empty attributes.
Tests added for HTML and YAML generators, merging and serializing.
computeRelativePath function had a bug when the filepath is part of the given 
directory; it returned a path that starts with a separator. This has been fixed.


https://reviews.llvm.org/D65987

Files:
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/MergeTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -29,8 +29,9 @@
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
   I.ChildNamespaces.emplace_back(EmptySID, "ChildNamespace",
- InfoType::IT_namespace);
-  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record);
+ InfoType::IT_namespace, "path/to/A/Namespace");
+  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+  "path/to/A/Namespace");
   I.ChildFunctions.emplace_back();
   I.ChildFunctions.back().Name = "OneFunction";
   I.ChildEnums.emplace_back();
@@ -53,9 +54,11 @@
 ChildNamespaces:
   - Type:Namespace
 Name:'ChildNamespace'
+Path:'path/to/A/Namespace'
 ChildRecords:
   - Type:Record
 Name:'ChildStruct'
+Path:'path/to/A/Namespace'
 ChildFunctions:
   - USR: ''
 Name:'OneFunction'
@@ -71,7 +74,7 @@
 TEST(YAMLGeneratorTest, emitRecordYAML) {
   RecordInfo I;
   I.Name = "r";
-  I.Path = "path/to/r";
+  I.Path = "path/to/A";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
@@ -85,7 +88,8 @@
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record,
 "path/to/G");
 
-  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record);
+  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+  "path/to/A/r");
   I.ChildFunctions.emplace_back();
   I.ChildFunctions.back().Name = "OneFunction";
   I.ChildEnums.emplace_back();
@@ -101,7 +105,7 @@
   R"raw(---
 USR: ''
 Name:'r'
-Path:'path/to/r'
+Path:'path/to/A'
 Namespace:
   - Type:Namespace
 Name:'A'
@@ -129,6 +133,7 @@
 ChildRecords:
   - Type:Record
 Name:'ChildStruct'
+Path:'path/to/A/r'
 ChildFunctions:
   - USR: ''
 Name:'OneFunction'
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -407,12 +407,14 @@
 
   RecordInfo *ParentB = InfoAsRecord(Infos[3].get());
   RecordInfo ExpectedParentB(EmptySID);
-  ExpectedParentB.ChildRecords.emplace_back(EmptySID, "B", InfoType::IT_record);
+  ExpectedParentB.ChildRecords.emplace_back(EmptySID, "B", InfoType::IT_record,
+"A");
   CheckRecordInfo(, ParentB);
 
   NamespaceInfo *ParentC = InfoAsNamespace(Infos[7].get());
   NamespaceInfo ExpectedParentC(EmptySID);
-  ExpectedParentC.ChildRecords.emplace_back(EmptySID, "C", InfoType::IT_record);
+  ExpectedParentC.ChildRecords.emplace_back(EmptySID, "C", InfoType::IT_record,
+"@nonymous_namespace");
   CheckNamespaceInfo(, ParentC);
 }
 
@@ -431,7 +433,7 @@
   NamespaceInfo *ParentB = InfoAsNamespace(Infos[3].get());
   NamespaceInfo ExpectedParentB(EmptySID);
   ExpectedParentB.ChildNamespaces.emplace_back(EmptySID, "B",
-   InfoType::IT_namespace);
+   InfoType::IT_namespace, "A");
   

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:172
+  case VFSMode::Unknown:
+if (!this->VFS) {
+  LLVM_FALLTHROUGH;

Is this really necessary? 



Comment at: clang/lib/Driver/Driver.cpp:980
 
+  // We might try accessing the VFS fairly eatly, so make sure we have the
+  // right one.

`s/eatly/early/`



Comment at: clang/test/Driver/vfsmode.py:18
+current = os.path.realpath(os.curdir)
+name_max = int(subprocess.check_output("getconf NAME_MAX /", shell=True))
+path_max = int(subprocess.check_output("getconf PATH_MAX /", shell=True))

Can we pass `current` here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65986



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

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:167
+else
+  Diag(diag::err_drv_unsupported_option_argument) << OptionPrefix << Value;
+  }

It might be nice to `break` here to exit early, unless you want to make Clang 
pick the last one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65986



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

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

[Amy Huang via Phabricator via cfe-commits]

akhuang added a comment.

> Can you post a reproducer?

Turns out I just didn't have assertions enabled. With assertions the changed 
test cases should fail.

> I think this is precisely what was discussed in replies to RFC - this 
> hardcodes these address spaces, and thus either makes them unavaliable for 
> other front-ends and/or forces them to use them with Precisely Same Meaning.

It seems like the RFC replies agreed that this should be implemented with 
address spaces, and that the information for these is encoded in the data 
layout. I think there was some confusion as to whether there would be more 
changes in addition to the data layout change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64931



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

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

[JF Bastien via Phabricator via cfe-commits]

jfb marked an inline comment as done.
jfb added inline comments.



Comment at: clang/test/Driver/vfsmode.py:4
+
+# UNSUPPORTED: system-windows
+

I'm not sure what the best way to test this on Windows would be, and without a 
machine handy I can't really test this :-(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65986



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

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

[JF Bastien via Phabricator via cfe-commits]

jfb created this revision.
jfb added reviewers: JDevlieghere, bruno.
Herald added subscribers: cfe-commits, dexonsmith, jkorous.
Herald added a project: clang.

The motivation for 'physical' mode in D58169  
was pretty sensible, but it has one unfortunate side-effect: working close to 
the maximum path size now causes failures when it used not to. We therefore 
want to be able to set the VFS mode for the driver. This requires moving where 
the driver's VFS is initialized because arguments aren't available in the 
Driver constructor. I was careful to put this initialization early enough that 
the VFS isn't used yet (it would segfault if it were, since it's not set).

rdar://problem/54103540


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65986

Files:
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/vfsmode.py

Index: clang/test/Driver/vfsmode.py
===
--- /dev/null
+++ clang/test/Driver/vfsmode.py
@@ -0,0 +1,49 @@
+# Check that we can change the VFS mode between 'real' and 'physical', which
+# affects the maximum path length that clang can deal with.
+
+# UNSUPPORTED: system-windows
+
+# RUN: rm -rf xxx*
+# RUN: python %s > %t
+# RUN: cat %t | xargs not %clang 2>&1 | FileCheck %s --check-prefix=PHYSICAL
+# RUN: cat %t | xargs not %clang -ivfsmode=physical  2>&1 | FileCheck %s --check-prefix=PHYSICAL
+# RUN: cat %t | xargs %clang -ivfsmode=real
+
+# PHYSICAL: error: no such file or directory:
+
+import os
+import subprocess
+
+current = os.path.realpath(os.curdir)
+name_max = int(subprocess.check_output("getconf NAME_MAX /", shell=True))
+path_max = int(subprocess.check_output("getconf PATH_MAX /", shell=True))
+assert name_max > 0
+assert path_max > 0
+
+filename = 'o.c'
+code = "int main() { return 0; }"
+
+relative = ''
+absolute = current
+
+# Create directories which almost, but not quite, exhaust PATH_MAX.
+target_len = path_max - 1
+while len(absolute) < target_len:
+new_name_len = target_len - len(absolute) - 1
+new_name_len = new_name_len if new_name_len < name_max else name_max
+absolute = os.path.join(absolute, 'x' * new_name_len)
+relative = os.path.join(relative, 'x' * new_name_len)
+os.mkdir(relative)
+assert os.path.exists(absolute)
+assert os.path.exists(relative)
+
+# Create a file whose relative path doesn't exceed PATH_MAX, but whose absolute
+# path does.
+file_relative_path = os.path.join(relative, filename)
+with open(file_relative_path, 'w') as f:
+f.write(code)
+
+assert os.path.exists(file_relative_path)
+
+# Print out the relative path so lit can pass to clang.
+print file_relative_path
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -131,10 +131,6 @@
   CCCGenericGCCName(""), Saver(Alloc), CheckInputsExist(true),
   GenReproducer(false), SuppressMissingInputWarning(false) {
 
-  // Provide a sane fallback if no VFS is specified.
-  if (!this->VFS)
-this->VFS = llvm::vfs::createPhysicalFileSystem().release();
-
   Name = llvm::sys::path::filename(ClangExecutable);
   Dir = llvm::sys::path::parent_path(ClangExecutable);
   InstalledDir = Dir; // Provide a sensible default installed dir.
@@ -150,6 +146,41 @@
   ResourceDir = GetResourcesPath(ClangExecutable, CLANG_RESOURCE_DIR);
 }
 
+void Driver::ParseVFSMode(ArrayRef Args) {
+  const std::string OptionPrefix =
+  getOpts().getOption(options::OPT_ivfsmode).getPrefixedName();
+  enum class VFSMode { Unknown, Real, Physical } Mode = VFSMode::Unknown;
+  for (const char *ArgPtr : Args) {
+if (!ArgPtr)
+  continue;
+const StringRef Arg = ArgPtr;
+if (!Arg.startswith(OptionPrefix))
+  continue;
+const StringRef Value = Arg.drop_front(OptionPrefix.size());
+VFSMode M = llvm::StringSwitch(Value)
+.Case("real", VFSMode::Real)
+.Case("physical", VFSMode::Physical)
+.Default(VFSMode::Unknown);
+if (M != VFSMode::Unknown)
+  Mode = M;
+else
+  Diag(diag::err_drv_unsupported_option_argument) << OptionPrefix << Value;
+  }
+
+  switch (Mode) {
+  case VFSMode::Unknown:
+if (!this->VFS) {
+  LLVM_FALLTHROUGH;
+case VFSMode::Physical:
+  this->VFS = llvm::vfs::createPhysicalFileSystem().release();
+}
+break;
+  case VFSMode::Real:
+this->VFS = llvm::vfs::getRealFileSystem().get();
+break;
+  }
+}
+
 void Driver::ParseDriverMode(StringRef ProgramName,
  ArrayRef Args) {
   if (ClangNameParts.isEmpty())
@@ -946,6 +977,10 @@
 }
   }
 
+  // We might try accessing the VFS fairly eatly, so make sure we have the
+  // right one.
+  ParseVFSMode(ArgList.slice(1));
+
   // We look for the driver mode option 

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

[Jian Cai via Phabricator via cfe-commits]

jcai19 marked an inline comment as done.
jcai19 added inline comments.



Comment at: clang/lib/Basic/Targets/ARM.cpp:325
+   ? "llvm.arm.gnu.eabi.mcount"
: "\01mcount";
 

nickdesaulniers wrote:
> Doesn't require changes, but for anyone curious about the `\01`, see the 
> comment in `MangleContext::mangleName`.
Thanks for the reference!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65019



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

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

[Jian Cai via Phabricator via cfe-commits]

jcai19 updated this revision to Diff 214263.
jcai19 added a comment.

Lower the new intrinsic when legalizing DAGs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65019

Files:
  clang/lib/Basic/Targets/ARM.cpp
  llvm/include/llvm/IR/IntrinsicsARM.td
  llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/ARM/ARMISelLowering.h
  llvm/lib/Target/ARM/ARMInstrInfo.td
  llvm/lib/Target/ARM/ARMInstrThumb.td
  llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
  llvm/test/CodeGen/ARM/gnu_mcount_nc.ll

Index: llvm/test/CodeGen/ARM/gnu_mcount_nc.ll
===
--- /dev/null
+++ llvm/test/CodeGen/ARM/gnu_mcount_nc.ll
@@ -0,0 +1,21 @@
+; RUN: llc -mtriple=armv7a-linux-gnueabihf %s -o - | FileCheck %s --check-prefix=CHECK-ARM
+; RUN: llc -mtriple=thumbv7a-linux-gnueabihf %s -o - | FileCheck %s --check-prefix=CHECK-THUMB
+
+define dso_local void @callee() #0 {
+; CHECK-ARM:stmdb   sp!, {lr}
+; CHECK-ARM-NEXT:   bl  __gnu_mcount_nc
+; CHECK-THUMB:  push{lr}
+; CHECK-THUMB-NEXT: bl  __gnu_mcount_nc
+  ret void
+}
+
+define dso_local void @caller() #0 {
+; CHECK-ARM:stmdb   sp!, {lr}
+; CHECK-ARM-NEXT:   bl  __gnu_mcount_nc
+; CHECK-THUMB:  push{lr}
+; CHECK-THUMB-NEXT: bl  __gnu_mcount_nc
+  call void @callee()
+  ret void
+}
+
+attributes #0 = { nofree nounwind "instrument-function-entry-inlined"="llvm.arm.gnu.eabi.mcount" }
Index: llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
===
--- llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
+++ llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
@@ -24,7 +24,7 @@
 
   if (Func == "mcount" ||
   Func == ".mcount" ||
-  Func == "\01__gnu_mcount_nc" ||
+  Func == "llvm.arm.gnu.eabi.mcount" ||
   Func == "\01_mcount" ||
   Func == "\01mcount" ||
   Func == "__mcount" ||
Index: llvm/lib/Target/ARM/ARMInstrThumb.td
===
--- llvm/lib/Target/ARM/ARMInstrThumb.td
+++ llvm/lib/Target/ARM/ARMInstrThumb.td
@@ -565,6 +565,13 @@
   4, IIC_Br,
   [(ARMcall_nolink tGPR:$func)]>,
 Requires<[IsThumb, IsThumb1Only]>, Sched<[WriteBr]>;
+
+  // Also used for Thumb2
+  // push lr before the call
+  def tBL_PUSHLR : tPseudoInst<(outs), (ins pred:$p, thumb_bl_target:$func),
+  4, IIC_Br,
+  [(ARMcall_pushlr texternalsym:$func)]>,
+ Requires<[IsThumb]>, Sched<[WriteBr]>;
 }
 
 let isBranch = 1, isTerminator = 1, isBarrier = 1 in {
Index: llvm/lib/Target/ARM/ARMInstrInfo.td
===
--- llvm/lib/Target/ARM/ARMInstrInfo.td
+++ llvm/lib/Target/ARM/ARMInstrInfo.td
@@ -148,6 +148,9 @@
 def ARMcall_nolink   : SDNode<"ARMISD::CALL_NOLINK", SDT_ARMcall,
   [SDNPHasChain, SDNPOptInGlue, SDNPOutGlue,
SDNPVariadic]>;
+def ARMcall_pushlr : SDNode<"ARMISD::CALL_PUSHLR", SDT_ARMcall,
+  [SDNPHasChain, SDNPOptInGlue, SDNPOutGlue,
+   SDNPVariadic]>;
 
 def ARMretflag   : SDNode<"ARMISD::RET_FLAG", SDTNone,
   [SDNPHasChain, SDNPOptInGlue, SDNPVariadic]>;
@@ -2350,6 +2353,12 @@
   def BMOVPCB_CALL : ARMPseudoInst<(outs), (ins arm_bl_target:$func),
8, IIC_Br, [(ARMcall_nolink tglobaladdr:$func)]>,
   Requires<[IsARM]>, Sched<[WriteBr]>;
+
+  // push lr before the call
+  def BL_PUSHLR : ARMPseudoInst<(outs), (ins arm_bl_target:$func),
+  4, IIC_Br,
+  [(ARMcall_pushlr texternalsym:$func)]>,
+ Requires<[IsARM]>, Sched<[WriteBr]>;
 }
 
 let isBranch = 1, isTerminator = 1 in {
Index: llvm/lib/Target/ARM/ARMISelLowering.h
===
--- llvm/lib/Target/ARM/ARMISelLowering.h
+++ llvm/lib/Target/ARM/ARMISelLowering.h
@@ -68,6 +68,7 @@
   CALL, // Function call.
   CALL_PRED,// Function call that's predicable.
   CALL_NOLINK,  // Function call with branch not branch-and-link.
+  CALL_PUSHLR,  // Function call that pushes LR before the call.
   BRCOND,   // Conditional branch.
   BR_JT,// Jumptable branch.
   BR2_JT,   // Jumptable branch (2 level - jumptable entry is a jump).
@@ -666,6 +667,8 @@
 SDValue LowerEH_SJLJ_SETJMP(SDValue Op, SelectionDAG ) const;
 SDValue LowerEH_SJLJ_LONGJMP(SDValue Op, SelectionDAG ) const;
 SDValue LowerEH_SJLJ_SETUP_DISPATCH(SDValue Op, SelectionDAG ) const;
+SDValue LowerINTRINSIC_VOID(SDValue Op, SelectionDAG ,
+const ARMSubtarget *Subtarget) const;
  

[PATCH] D65835: [OpenMP] Fix map/is_device_ptr with DSA on combined directive

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

> I want to be sure we're on the same page: For OpenMP 5.0, should we allow 
> is_device_ptr with the private clauses?

Yes, since it is allowed by the standard.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65835



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

[PATCH] D65919: [clang-tidy] Add llvm-prefer-register-over-unsigned check and apply it to LLVM

[Daniel Sanders via Phabricator via cfe-commits]

dsanders marked an inline comment as done.
dsanders added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp:53
+  }
+  diag(UserVarDecl->getLocation(), "use '%0'", DiagnosticIDs::Note)
+  << Replacement

aaron.ballman wrote:
> I don't think you should issue a second diagnostic on the same line. Instead, 
> only issue the previous diagnostic with the fixit attached to it.
I don't mind changing this but I thought I should mention that I'm following 
the example set by the code generated by add_new_check.py which has the 
diagnostic separate from the note with the fixit:
```
  diag(MatchedDecl->getLocation(), "function %%0 is insufficiently awesome")
  << MatchedDecl;
  diag(MatchedDecl->getLocation(), "insert 'awesome'", DiagnosticIDs::Note)
  << FixItHint::CreateInsertion(MatchedDecl->getLocation(), "awesome_");
```
Is the example doing it the right way?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65919



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

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D64931#1622038 , @akhuang wrote:

> @lebedev.ri The test case datalayout strings were changed because somewhere 
> llvm asserts that the string in the IR matches the backend datalayout. I 
> don't know why I wasn't getting the assert error now, but I think they'll all 
> have to be changed


Can you post a reproducer?

In D64931#1622038 , @akhuang wrote:

> if we change the X86 datalayout?


I think this is precisely what was discussed in replies to RFC - this hardcodes 
these address spaces,
and thus either makes them unavaliable for other front-ends and/or forces them 
to use them with Precisely Same Meaning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64931



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

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

[Amy Huang via Phabricator via cfe-commits]

akhuang added a comment.

@lebedev.ri The test case datalayout strings were changed because somewhere 
llvm asserts that the string in the IR matches the backend datalayout. I don't 
know why I wasn't getting the assert error now, but I think they'll all have to 
be changed if we change the X86 datalayout?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64931



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

[PATCH] D65835: [OpenMP] Fix map/is_device_ptr with DSA on combined directive

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D65835#1621202 , @ABataev wrote:

> >> See 2.19.7 Data-Mapping Attribute Rules, Clauses, and Directives
> > 
> > I looked again.  I'm still not finding any text in that section that 
> > implies is_device_ptr follows the same restrictions as map.  Can you please 
> > cite specific lines of text instead of an entire section?  Thanks for your 
> > help.
>
> Ah, it is only in OpenMP 5.0 anв restrictions for the map clause are for map 
> clause only. Then we should allow `is_device_ptr` with the private clauses 
> for OpenMP 4.5.


I want to be sure we're on the same page: For OpenMP 5.0, should we allow 
`is_device_ptr` with the private clauses?

> Better to do this in a separate patch only for `is_device_ptr`.

I'll remove it from this one.  Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65835



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

[PATCH] D65974: Driver: Don't warn about assembler flags being unused when not assembling AND not using the integrated assembler

[Douglas Yung via Phabricator via cfe-commits]

dyung marked an inline comment as done.
dyung added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3559
 // CollectArgsForIntegratedAssembler().
-if (TC.useIntegratedAs()) {
+if (TC.useIntegratedAs() || isa(JA)) {
   Args.ClaimAllArgs(options::OPT_mrelax_all);

thakis wrote:
> We shouldn't claim the OPT_m flags if we don't use integrated-as. Claiming 
> Wa_COMMA and Xassembler makes sense to me.
If we do not, then one of the tests you included in this change will fail if a 
platform defaults to not using the integrated assembler. Should I remove this 
change and just update the test to explicitly set 
-fintegrated-as/-fno-integrated-as and test for the expected result?


Repository:
  rC Clang

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

https://reviews.llvm.org/D65974



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

[PATCH] D65978: [clang] Fixed x86 cpuid NSC signature

[Craig Topper via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D65978



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

Re: r368354 - Mark clang-scan-deps test as requiring thread support

[Alex L via cfe-commits]

Thanks for fixing this!

I think changing clang-scan-deps to ignore -j when `LLVM_ENABLE_THREADS` is
probably a better solution. I'll work on a patch that does that.



On Thu, 8 Aug 2019 at 15:07, Reid Kleckner  wrote:

> The specific issue here is that clang-scan-deps uses threads, which seems
> to work just fine. But, it calls some code that sets up PrettyStackTrace
> RAII objects, which normally use TLS. And when LLVM_ENABLE_THREADS is off,
> LLVM_THREAD_LOCAL expands to nothing, so the TLS variables are simply
> global, and the RAII objects assert that things haven't been constructed
> and destructed in the correct order.
>
> So, going forward you will probably need to remember to add REQUIRES:
> thread_support to individual tests, or change clang-scan-deps to ignore the
> -j parameter when threads have been disabled.
>
> On Thu, Aug 8, 2019 at 2:45 PM Reid Kleckner via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: rnk
>> Date: Thu Aug  8 14:45:59 2019
>> New Revision: 368354
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=368354=rev
>> Log:
>> Mark clang-scan-deps test as requiring thread support
>>
>> Otherwise the test calls a pure virtual method and crashes. Perhaps this
>> could be improved.
>>
>> Modified:
>> cfe/trunk/test/ClangScanDeps/regular_cdb.cpp
>>
>> Modified: cfe/trunk/test/ClangScanDeps/regular_cdb.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ClangScanDeps/regular_cdb.cpp?rev=368354=368353=368354=diff
>>
>> ==
>> --- cfe/trunk/test/ClangScanDeps/regular_cdb.cpp (original)
>> +++ cfe/trunk/test/ClangScanDeps/regular_cdb.cpp Thu Aug  8 14:45:59 2019
>> @@ -1,3 +1,4 @@
>> +// REQUIRES: thread_support
>>  // RUN: rm -rf %t.dir
>>  // RUN: rm -rf %t.cdb
>>  // RUN: mkdir -p %t.dir
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r368354 - Mark clang-scan-deps test as requiring thread support

[Reid Kleckner via cfe-commits]

The specific issue here is that clang-scan-deps uses threads, which seems
to work just fine. But, it calls some code that sets up PrettyStackTrace
RAII objects, which normally use TLS. And when LLVM_ENABLE_THREADS is off,
LLVM_THREAD_LOCAL expands to nothing, so the TLS variables are simply
global, and the RAII objects assert that things haven't been constructed
and destructed in the correct order.

So, going forward you will probably need to remember to add REQUIRES:
thread_support to individual tests, or change clang-scan-deps to ignore the
-j parameter when threads have been disabled.

On Thu, Aug 8, 2019 at 2:45 PM Reid Kleckner via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rnk
> Date: Thu Aug  8 14:45:59 2019
> New Revision: 368354
>
> URL: http://llvm.org/viewvc/llvm-project?rev=368354=rev
> Log:
> Mark clang-scan-deps test as requiring thread support
>
> Otherwise the test calls a pure virtual method and crashes. Perhaps this
> could be improved.
>
> Modified:
> cfe/trunk/test/ClangScanDeps/regular_cdb.cpp
>
> Modified: cfe/trunk/test/ClangScanDeps/regular_cdb.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ClangScanDeps/regular_cdb.cpp?rev=368354=368353=368354=diff
>
> ==
> --- cfe/trunk/test/ClangScanDeps/regular_cdb.cpp (original)
> +++ cfe/trunk/test/ClangScanDeps/regular_cdb.cpp Thu Aug  8 14:45:59 2019
> @@ -1,3 +1,4 @@
> +// REQUIRES: thread_support
>  // RUN: rm -rf %t.dir
>  // RUN: rm -rf %t.cdb
>  // RUN: mkdir -p %t.dir
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65974: Driver: Don't warn about assembler flags being unused when not assembling AND not using the integrated assembler

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3559
 // CollectArgsForIntegratedAssembler().
-if (TC.useIntegratedAs()) {
+if (TC.useIntegratedAs() || isa(JA)) {
   Args.ClaimAllArgs(options::OPT_mrelax_all);

We shouldn't claim the OPT_m flags if we don't use integrated-as. Claiming 
Wa_COMMA and Xassembler makes sense to me.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65974



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

[PATCH] D65978: [clang] Fixed x86 cpuid NSC signature

[Louis Jacotot via Phabricator via cfe-commits]

Jacotot created this revision.
Jacotot added reviewers: teemperor, rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The signature "Geode by NSC" for NSC vendor is wrong.
In lib/Headers/cpuid.h, signature_NSC_edx and signature_NSC_ecx constants are 
inverted (cpuid signature order is ebx # edx # ecx).


Repository:
  rC Clang

https://reviews.llvm.org/D65978

Files:
  clang/lib/Headers/cpuid.h


Index: clang/lib/Headers/cpuid.h
===
--- clang/lib/Headers/cpuid.h
+++ clang/lib/Headers/cpuid.h
@@ -38,8 +38,8 @@
 #define signature_TM2_ecx 0x3638784d
 /* NSC: "Geode by NSC" */
 #define signature_NSC_ebx 0x646f6547
-#define signature_NSC_edx 0x43534e20
-#define signature_NSC_ecx 0x79622065
+#define signature_NSC_edx 0x79622065
+#define signature_NSC_ecx 0x43534e20
 /* NEXGEN:  "NexGenDriven" */
 #define signature_NEXGEN_ebx 0x4778654e
 #define signature_NEXGEN_edx 0x72446e65


Index: clang/lib/Headers/cpuid.h
===
--- clang/lib/Headers/cpuid.h
+++ clang/lib/Headers/cpuid.h
@@ -38,8 +38,8 @@
 #define signature_TM2_ecx 0x3638784d
 /* NSC: "Geode by NSC" */
 #define signature_NSC_ebx 0x646f6547
-#define signature_NSC_edx 0x43534e20
-#define signature_NSC_ecx 0x79622065
+#define signature_NSC_edx 0x79622065
+#define signature_NSC_ecx 0x43534e20
 /* NEXGEN:  "NexGenDriven" */
 #define signature_NEXGEN_ebx 0x4778654e
 #define signature_NEXGEN_edx 0x72446e65
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r368354 - Mark clang-scan-deps test as requiring thread support

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Thu Aug  8 14:45:59 2019
New Revision: 368354

URL: http://llvm.org/viewvc/llvm-project?rev=368354=rev
Log:
Mark clang-scan-deps test as requiring thread support

Otherwise the test calls a pure virtual method and crashes. Perhaps this
could be improved.

Modified:
cfe/trunk/test/ClangScanDeps/regular_cdb.cpp

Modified: cfe/trunk/test/ClangScanDeps/regular_cdb.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ClangScanDeps/regular_cdb.cpp?rev=368354=368353=368354=diff
==
--- cfe/trunk/test/ClangScanDeps/regular_cdb.cpp (original)
+++ cfe/trunk/test/ClangScanDeps/regular_cdb.cpp Thu Aug  8 14:45:59 2019
@@ -1,3 +1,4 @@
+// REQUIRES: thread_support
 // RUN: rm -rf %t.dir
 // RUN: rm -rf %t.cdb
 // RUN: mkdir -p %t.dir


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

r368348 - Fix up fd limit diagnosis code

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Thu Aug  8 14:35:03 2019
New Revision: 368348

URL: http://llvm.org/viewvc/llvm-project?rev=368348=rev
Log:
Fix up fd limit diagnosis code

Apparently Windows returns the "invalid argument" error code when the
path contains invalid characters such as '<'. The
test/Preprocessor/include-likely-typo.c test does this, so it was
failing after r368322.

Also, the diagnostic requires two arguments, so add the filename.

Modified:
cfe/trunk/lib/Lex/HeaderSearch.cpp

Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=368348=368347=368348=diff
==
--- cfe/trunk/lib/Lex/HeaderSearch.cpp (original)
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp Thu Aug  8 14:35:03 2019
@@ -316,8 +316,9 @@ const FileEntry *HeaderSearch::getFileAn
 // message.
 std::error_code EC = File.getError();
 if (EC != std::errc::no_such_file_or_directory &&
-EC != std::errc::is_a_directory) {
-  Diags.Report(IncludeLoc, diag::err_cannot_open_file) << EC.message();
+EC != std::errc::invalid_argument && EC != std::errc::is_a_directory) {
+  Diags.Report(IncludeLoc, diag::err_cannot_open_file)
+  << FileName << EC.message();
 }
 return nullptr;
   }


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

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:26
 class CastValueChecker : public Checker {
+  enum class CastKind { Checking, Sugar };
+

NoQ wrote:
> Please explain "Checking" and "Sugar". Checking what? Sugar... you mean 
> syntactic sugar? For what?
I have no idea what devs mean by those names:
- `Checking` kind:
> The cast<> operator is a “checked cast” operation.
> The dyn_cast<> operator is a “checking cast” operation.
from 
http://llvm.org/docs/ProgrammersManual.html#the-isa-cast-and-dyn-cast-templates

- `Sugar` kind:
> Member-template castAs. Look through sugar for the underlying 
> instance of .
> Member-template getAs'. Look through sugar for an instance of 
> .
from 
https://clang.llvm.org/doxygen/classclang_1_1Type.html#a436b8b08ae7f2404b4712d37986194ce
and 
https://clang.llvm.org/doxygen/classclang_1_1Type.html#adae68e1f4c85ede2d36da45fbefc48a2

- `isa()` would be `Instance` kind:
> The isa<> operator works exactly like the Java “instanceof” operator.
from 
http://llvm.org/docs/ProgrammersManual.html#the-isa-cast-and-dyn-cast-templates

If you could imagine better names, please let me know. I have tried to use the 
definitions.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:94
+  [CastFromName, CastToName, IsNullReturn,
+   IsSimpleCast](BugReport &) -> std::string {
 SmallString<128> Msg;

NoQ wrote:
> `IsDynamicCast`.
It is not `dyn_cast` but simple `cast`. Because we have decided to use 
`Checked` for that cast, I believe `IsCheckedCast` the one.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:98
 
-Out << "Assuming dynamic cast from '" << CastFromName << "' to '"
-<< CastToName << "' succeeds";
-return Out.str();
-  },
-  /*IsPrunable=*/true);
+Out << (!IsSimpleCast ? "Assuming dynamic cast " : "Dynamic cast ");
+if (CastFromName)

NoQ wrote:
> More suggestions for the `:`-branch: "Hard cast" (a bit too jargon-y), "Safe 
> cast", "Checked cast".
Well, let us pick `Checked` then as that is the proper definition for `cast()`, 
but that other sugar-based `castAs()` definition makes me mad.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:220
 
-  // If we cannot obtain both of the classes we cannot be sure how to model it.
-  if (!CE->getType()->getPointeeCXXRecordDecl() ||
-  !CE->getArg(0)->getType()->getPointeeCXXRecordDecl())
-return false;
+  const CastCheck *Check = >first;
+  CastKind Kind = Lookup->second;

NoQ wrote:
> What's the point of making this a raw pointer? (no pun intended)
I just wanted to make it usable as it being used since the first review as 
`(*Check)` and emphasize it is not a given function-call but a pointer to one. 
If you think as a nude `Check()` it is fine, then I have no idea which is 
better and it is fine for me as well.



Comment at: clang/test/Analysis/cast-value.cpp:156-167
+void evalNonNullParamNonNullReturn(const Shape *S) {
+  const auto *C = cast(S);
+  // expected-note@-1 {{Dynamic cast from 'Shape' to 'Circle' succeeds}}
+  // expected-note@-2 {{Assuming pointer value is null}}
+  // expected-note@-3 {{'C' initialized here}}
+
+  (void)(1 / !(bool)C);

NoQ wrote:
> Mmm, wait a sec. That's a false positive. `cast<>` doesn't accept null 
> pointers. We have `cast_or_null` for this.
This `Assuming pointer value is null` note is very random. I believe it is not 
made by me and my code is fine, so I have printed a graph:
{F9759380}
Do you see any problem?


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

https://reviews.llvm.org/D65889



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

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 214234.
Charusso marked 7 inline comments as done.
Charusso added a comment.

- Added a test case for casting *to* a reference.
- Better naming.


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

https://reviews.llvm.org/D65889

Files:
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/test/Analysis/cast-value.cpp

Index: clang/test/Analysis/cast-value.cpp
===
--- clang/test/Analysis/cast-value.cpp
+++ clang/test/Analysis/cast-value.cpp
@@ -14,20 +14,33 @@
 const X *cast(Y Value);
 
 template 
-const X *dyn_cast(Y Value);
+const X *dyn_cast(Y *Value);
+template 
+const X _cast(Y );
 
 template 
 const X *cast_or_null(Y Value);
 
 template 
-const X *dyn_cast_or_null(Y Value);
+const X *dyn_cast_or_null(Y *Value);
+template 
+const X *dyn_cast_or_null(Y );
 } // namespace llvm
 
-using namespace llvm;
+namespace clang {
+struct Shape {
+  template 
+  const T *castAs() const;
 
-class Shape {};
+  template 
+  const T *getAs() const;
+};
 class Triangle : public Shape {};
 class Circle : public Shape {};
+} // namespace clang
+
+using namespace llvm;
+using namespace clang;
 
 namespace test_cast {
 void evalLogic(const Shape *S) {
@@ -91,8 +104,52 @@
   if (!S)
 clang_analyzer_eval(!C); // logic-warning {{TRUE}}
 }
+} // namespace test_dyn_cast_or_null
 
-void evalNonNullParamNonNullReturn(const Shape *S) {
+namespace test_cast_as {
+void evalLogic(const Shape *S) {
+  const Circle *C = S->castAs();
+  clang_analyzer_numTimesReached(); // logic-warning {{1}}
+
+  if (S && C)
+clang_analyzer_eval(C == S);
+  // logic-warning@-1 {{TRUE}}
+
+  if (S && !C)
+clang_analyzer_warnIfReached(); // no-warning
+
+  if (!S)
+clang_analyzer_warnIfReached(); // no-warning
+}
+} // namespace test_cast_as
+
+namespace test_get_as {
+void evalLogic(const Shape *S) {
+  const Circle *C = S->getAs();
+  clang_analyzer_numTimesReached(); // logic-warning {{2}}
+
+  if (S && C)
+clang_analyzer_eval(C == S);
+  // logic-warning@-1 {{TRUE}}
+
+  if (S && !C)
+clang_analyzer_warnIfReached(); // logic-warning {{REACHABLE}}
+
+  if (!S)
+clang_analyzer_warnIfReached(); // no-warning
+}
+} // namespace test_get_as
+
+namespace test_notes {
+void evalReferences(const Shape ) {
+  const auto  = dyn_cast(S);
+  // expected-note@-1 {{Assuming dynamic cast from 'Shape' to 'Circle' fails}}
+  // expected-note@-2 {{Dereference of null pointer}}
+  // expected-warning@-3 {{Dereference of null pointer}}
+  // logic-warning@-4 {{Dereference of null pointer}}
+}
+
+void evalNonNullParamNonNullReturnReference(const Shape ) {
   const auto *C = dyn_cast_or_null(S);
   // expected-note@-1 {{Assuming dynamic cast from 'Shape' to 'Circle' succeeds}}
   // expected-note@-2 {{Assuming pointer value is null}}
@@ -105,6 +162,19 @@
   // logic-warning@-4 {{Division by zero}}
 }
 
+void evalNonNullParamNonNullReturn(const Shape *S) {
+  const auto *C = cast(S);
+  // expected-note@-1 {{Checked cast from 'Shape' to 'Circle' succeeds}}
+  // expected-note@-2 {{Assuming pointer value is null}}
+  // expected-note@-3 {{'C' initialized here}}
+
+  (void)(1 / !(bool)C);
+  // expected-note@-1 {{'C' is non-null}}
+  // expected-note@-2 {{Division by zero}}
+  // expected-warning@-3 {{Division by zero}}
+  // logic-warning@-4 {{Division by zero}}
+}
+
 void evalNonNullParamNullReturn(const Shape *S) {
   const auto *C = dyn_cast_or_null(S);
   // expected-note@-1 {{Assuming dynamic cast from 'Shape' to 'Circle' fails}}
@@ -134,4 +204,40 @@
   // expected-warning@-2 {{Division by zero}}
   // logic-warning@-3 {{Division by zero}}
 }
-} // namespace test_dyn_cast_or_null
+
+void evalZeroParamNonNullReturnPointer(const Shape *S) {
+  const auto *C = S->castAs();
+  // expected-note@-1 {{Assuming pointer value is null}}
+  // expected-note@-2 {{Checked cast to 'Circle' succeeds}}
+  // expected-note@-3 {{'C' initialized here}}
+
+  (void)(1 / !(bool)C);
+  // expected-note@-1 {{'C' is non-null}}
+  // expected-note@-2 {{Division by zero}}
+  // expected-warning@-3 {{Division by zero}}
+  // logic-warning@-4 {{Division by zero}}
+}
+
+void evalZeroParamNonNullReturn(const Shape ) {
+  const auto *C = S.castAs();
+  // expected-note@-1 {{Checked cast to 'Circle' succeeds}}
+  // expected-note@-2 {{'C' initialized here}}
+
+  (void)(1 / !(bool)C);
+  // expected-note@-1 {{'C' is non-null}}
+  // expected-note@-2 {{Division by zero}}
+  // expected-warning@-3 {{Division by zero}}
+  // logic-warning@-4 {{Division by zero}}
+}
+
+void evalZeroParamNullReturn(const Shape ) {
+  const auto *C = S.getAs();
+  // expected-note@-1 {{Assuming dynamic cast to 'Circle' fails}}
+  // expected-note@-2 {{'C' initialized to a null pointer value}}
+
+  (void)(1 / (bool)C);
+  // expected-note@-1 {{Division by zero}}
+  // expected-warning@-2 {{Division by zero}}
+  // logic-warning@-3 {{Division by zero}}
+}
+} // namespace 

[PATCH] D65912: [clang-tidy] Add new check for math constants

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK added a comment.

The main reason why I've created this differential - asking to you about 
usefulness of this check for clang-tidy. I understand that there are a some 
TODO and formatting issues - it's ok for now.

As far I understand your remarks - you are not against this check so I can 
continue my work on this path. Great.

Let's discuss about some open questions:

1. Should we support C language and  header file? I think - yes, it's 
quite easy to implement.
2. Should we support converting to Boost.Constants? I don't think so.
3. In header file instead of defining math constants directly should we use 
them from  as much as possible?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65912



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

[PATCH] D65975: [NewPM][PassInstrumentation] IR printing support from clang driver

[Taewook Oh via Phabricator via cfe-commits]

twoh added a comment.

@fedor.sergeev @yamauchi I saw your discussions over llvm-dev mailing list 
regarding IR printing with the new pass manager, and though this might be the 
reason why IR printing is not supported under new PM with clang. I would 
appreciate if you can take a look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65975



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

[PATCH] D65975: [NewPM][PassInstrumentation] IR printing support from clang driver

[Taewook Oh via Phabricator via cfe-commits]

twoh created this revision.
twoh added reviewers: fedor.sergeev, philip.pfaffe.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

https://reviews.llvm.org/D50923 enabled the IR printing support for the new 
pass manager, but only for the case when `opt` tool is used as a driver. This 
patch is to enable the IR printing when `clang` is used as a driver.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65975

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/Misc/printer.c


Index: clang/test/Misc/printer.c
===
--- /dev/null
+++ clang/test/Misc/printer.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -emit-llvm -O2 -fexperimental-new-pass-manager -mllvm 
-print-before-all %s -o %t 2>&1 | FileCheck %s --check-prefix=CHECK-BEFORE
+// RUN: %clang_cc1 -emit-llvm -O2 -fexperimental-new-pass-manager -mllvm 
-print-after-all %s -o %t 2>&1 | FileCheck %s --check-prefix=CHECK-AFTER
+// CHECK-BEFORE: *** IR Dump Before ForceFunctionAttrsPass ***
+// CHECK-AFTER: *** IR Dump After ForceFunctionAttrsPass ***
+void foo() {}
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -37,6 +37,7 @@
 #include "llvm/MC/SubtargetFeature.h"
 #include "llvm/Passes/PassBuilder.h"
 #include "llvm/Passes/PassPlugin.h"
+#include "llvm/Passes/StandardInstrumentations.h"
 #include "llvm/Support/BuryPointer.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -1063,7 +1064,10 @@
   PTO.LoopVectorization = CodeGenOpts.VectorizeLoop;
   PTO.SLPVectorization = CodeGenOpts.VectorizeSLP;
 
-  PassBuilder PB(TM.get(), PTO, PGOOpt);
+  PassInstrumentationCallbacks PIC;
+  StandardInstrumentations SI;
+  SI.registerCallbacks(PIC);
+  PassBuilder PB(TM.get(), PTO, PGOOpt, );
 
   // Attempt to load pass plugins and register their callbacks with PB.
   for (auto  : CodeGenOpts.PassPlugins) {


Index: clang/test/Misc/printer.c
===
--- /dev/null
+++ clang/test/Misc/printer.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -emit-llvm -O2 -fexperimental-new-pass-manager -mllvm -print-before-all %s -o %t 2>&1 | FileCheck %s --check-prefix=CHECK-BEFORE
+// RUN: %clang_cc1 -emit-llvm -O2 -fexperimental-new-pass-manager -mllvm -print-after-all %s -o %t 2>&1 | FileCheck %s --check-prefix=CHECK-AFTER
+// CHECK-BEFORE: *** IR Dump Before ForceFunctionAttrsPass ***
+// CHECK-AFTER: *** IR Dump After ForceFunctionAttrsPass ***
+void foo() {}
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -37,6 +37,7 @@
 #include "llvm/MC/SubtargetFeature.h"
 #include "llvm/Passes/PassBuilder.h"
 #include "llvm/Passes/PassPlugin.h"
+#include "llvm/Passes/StandardInstrumentations.h"
 #include "llvm/Support/BuryPointer.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -1063,7 +1064,10 @@
   PTO.LoopVectorization = CodeGenOpts.VectorizeLoop;
   PTO.SLPVectorization = CodeGenOpts.VectorizeSLP;
 
-  PassBuilder PB(TM.get(), PTO, PGOOpt);
+  PassInstrumentationCallbacks PIC;
+  StandardInstrumentations SI;
+  SI.registerCallbacks(PIC);
+  PassBuilder PB(TM.get(), PTO, PGOOpt, );
 
   // Attempt to load pass plugins and register their callbacks with PB.
   for (auto  : CodeGenOpts.PassPlugins) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65974: Driver: Don't warn about assembler flags being unused when not assembling AND not using the integrated assembler

[Douglas Yung via Phabricator via cfe-commits]

dyung created this revision.
dyung added reviewers: thakis, rnk.
dyung added a project: clang.

A previous change D65233  (r367165) attempted 
to fix this issue, but it missed a case where the integrated assembler is not 
being used. For example, consider the case where clang is invoked with -E and 
also -fno-integrated-as, the compiler was not supressing the warnings about 
unused arguments. This change attempts to fix that as well as expand the 
testing added in the previous commit to cover both scenarios.


Repository:
  rC Clang

https://reviews.llvm.org/D65974

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/as-options.s


Index: clang/test/Driver/as-options.s
===
--- clang/test/Driver/as-options.s
+++ clang/test/Driver/as-options.s
@@ -39,30 +39,62 @@
 // Test that assembler options don't cause warnings when there's no assembler
 // stage.
 
-// RUN: %clang -mincremental-linker-compatible -E \
+// RUN: %clang -mincremental-linker-compatible -E -fintegrated-as \
 // RUN:   -o /dev/null -x c++ %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
-// RUN: %clang -mincremental-linker-compatible -E \
+// RUN: %clang -mincremental-linker-compatible -E -fno-integrated-as \
+// RUN:   -o /dev/null -x c++ %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
+
+// RUN: %clang -mincremental-linker-compatible -E -fintegrated-as \
 // RUN:   -o /dev/null -x assembler-with-cpp %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
+// RUN: %clang -mincremental-linker-compatible -E -fno-integrated-as \
+// RUN:   -o /dev/null -x assembler-with-cpp %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
+
 // RUN: %clang -mimplicit-it=always -target armv7-linux-gnueabi -E \
-// RUN:   -o /dev/null -x c++ %s 2>&1 \
+// RUN:   -fintegrated-as -o /dev/null -x c++ %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
 // RUN: %clang -mimplicit-it=always -target armv7-linux-gnueabi -E \
-// RUN:   -o /dev/null -x assembler-with-cpp %s 2>&1 \
+// RUN:   -fno-integrated-as -o /dev/null -x c++ %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
+
+// RUN: %clang -mimplicit-it=always -target armv7-linux-gnueabi -E \
+// RUN:   -fintegrated-as -o /dev/null -x assembler-with-cpp %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
+// RUN: %clang -mimplicit-it=always -target armv7-linux-gnueabi -E \
+// RUN:   -fno-integrated-as -o /dev/null -x assembler-with-cpp %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
-// RUN: %clang -Wa,-mbig-obj -target i386-pc-windows -E \
+
+// RUN: %clang -Wa,-mbig-obj -target i386-pc-windows -E -fintegrated-as \
+// RUN:   -o /dev/null -x c++ %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
+// RUN: %clang -Wa,-mbig-obj -target i386-pc-windows -E -fno-integrated-as \
 // RUN:   -o /dev/null -x c++ %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
-// RUN: %clang -Wa,-mbig-obj -target i386-pc-windows -E \
+
+// RUN: %clang -Wa,-mbig-obj -target i386-pc-windows -E -fintegrated-as \
 // RUN:   -o /dev/null -x assembler-with-cpp %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
-// RUN: %clang -Xassembler -mbig-obj -target i386-pc-windows -E \
+// RUN: %clang -Wa,-mbig-obj -target i386-pc-windows -E -fno-integrated-as \
+// RUN:   -o /dev/null -x assembler-with-cpp %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
+
+// RUN: %clang -Xassembler -mbig-obj -target i386-pc-windows -E 
-fintegrated-as \
 // RUN:   -o /dev/null -x c++ %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
 // RUN: %clang -Xassembler -mbig-obj -target i386-pc-windows -E \
+// RUN:   -fno-integrated-as -o /dev/null -x c++ %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
+
+// RUN: %clang -Xassembler -mbig-obj -target i386-pc-windows -E 
-fintegrated-as \
 // RUN:   -o /dev/null -x assembler-with-cpp %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
+// RUN: %clang -Xassembler -mbig-obj -target i386-pc-windows -E \
+// RUN:   -fno-integrated-as -o /dev/null -x assembler-with-cpp %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
+
 // NOWARN-NOT: unused
 
 // Test that unsupported arguments do not cause errors when -fno-integrated-as
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3556,7 +3556,7 @@
   if (!isa(JA)) {
 // The args claimed here should match the args used in
 // CollectArgsForIntegratedAssembler().
-if (TC.useIntegratedAs()) {
+if (TC.useIntegratedAs() || isa(JA)) {
   

[PATCH] D65969: [clang][NFC] Consolidating usage of "FinalPhase" in Driver::BuildActions

[Saleem Abdulrasool via Phabricator via cfe-commits]

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

Seems like it should be equivalent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65969



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

[PATCH] D65120: More warnings regarding gsl::Pointer and gsl::Owner attributes

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun marked 4 inline comments as done.
xazax.hun added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:6572
+return false;
+  return llvm::StringSwitch(Callee->getName())
+  .Cases("begin", "rbegin", "cbegin", "crbegin", true)

gribozavr wrote:
> xazax.hun wrote:
> > gribozavr wrote:
> > > Should we also check that `Callee->getParent` is an owner?
> > > 
> > > Otherwise the check would complain about `begin()` on, for example, a 
> > > `std::string_view`.
> > This is intentional. We only warn if the initialization chain can be 
> > tracked back to a temporary (or local in some cases) owner. 
> > So we warn for the following code:
> > ```
> > const char *trackThroughMultiplePointer() {
> >   return std::basic_string_view(std::basic_string()).begin(); 
> > // expected-warning {{returning address of local temporary object}}
> > }
> > ```
> Makes sense, but then we should still check that `Callee->getParent` is an 
> owner or a pointer.
Good point, done.


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

https://reviews.llvm.org/D65120



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

[PATCH] D65120: More warnings regarding gsl::Pointer and gsl::Owner attributes

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun updated this revision to Diff 214228.
xazax.hun added a comment.

- Fix review comments.


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

https://reviews.llvm.org/D65120

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/Analysis/inner-pointer.cpp
  clang/test/Sema/warn-lifetime-analysis-nocfg.cpp

Index: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
===
--- clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -2,7 +2,6 @@
 struct [[gsl::Owner(int)]] MyIntOwner {
   MyIntOwner();
   int *();
-  int *c_str() const;
 };
 
 struct [[gsl::Pointer(int)]] MyIntPointer {
@@ -52,16 +51,6 @@
   return t.releaseAsRawPointer(); // ok
 }
 
-int *danglingRawPtrFromLocal() {
-  MyIntOwner t;
-  return t.c_str(); // TODO
-}
-
-int *danglingRawPtrFromTemp() {
-  MyIntPointer p;
-  return p.toOwner().c_str(); // TODO
-}
-
 struct Y {
   int a[4];
 };
@@ -103,6 +92,12 @@
   return MyIntOwner{}; // expected-warning {{returning address of local temporary object}}
 }
 
+MyIntOwner makeTempOwner();
+
+MyIntPointer danglingGslPtrFromTemporary2() {
+  return makeTempOwner(); // expected-warning {{returning address of local temporary object}}
+}
+
 MyLongPointerFromConversion danglingGslPtrFromTemporaryConv() {
   return MyLongOwnerWithConversion{}; // expected-warning {{returning address of local temporary object}}
 }
@@ -124,12 +119,52 @@
   global2 = MyLongOwnerWithConversion{}; // TODO ?
 }
 
-struct IntVector {
-  int *begin();
-  int *end();
+namespace std {
+template 
+struct basic_iterator {};
+
+template 
+struct vector {
+  typedef basic_iterator iterator;
+  iterator begin();
+  T *data();
+};
+
+template
+struct basic_string {
+  const T *c_str() const;
+};
+
+template
+struct unique_ptr {
+  T *get() const;
 };
+}
 
 void modelIterators() {
-  int *it = IntVector{}.begin(); // TODO ?
+  std::vector::iterator it = std::vector().begin(); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
   (void)it;
 }
+
+std::vector::iterator modelIteratorReturn() {
+  return std::vector().begin(); // expected-warning {{returning address of local temporary object}}
+}
+
+const char *danglingRawPtrFromLocal() {
+  std::basic_string s;
+  return s.c_str(); // expected-warning {{address of stack memory associated with local variable 's' returned}}
+}
+
+const char *danglingRawPtrFromTemp() {
+  return std::basic_string().c_str(); // expected-warning {{returning address of local temporary object}}
+}
+
+std::unique_ptr getUniquePtr();
+
+int *danglingUniquePtrFromTemp() {
+  return getUniquePtr().get(); // expected-warning {{returning address of local temporary object}}
+}
+
+int *danglingUniquePtrFromTemp2() {
+  return std::unique_ptr().get(); // expected-warning {{returning address of local temporary object}}
+}
Index: clang/test/Analysis/inner-pointer.cpp
===
--- clang/test/Analysis/inner-pointer.cpp
+++ clang/test/Analysis/inner-pointer.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.InnerPointer \
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.InnerPointer   \
+// RUN:   -Wno-dangling -Wno-dangling-field -Wno-return-stack-address \
 // RUN:   %s -analyzer-output=text -verify
 
 #include "Inputs/system-header-simulator-cxx.h"
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -6564,6 +6564,25 @@
   return false;
 }
 
+static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) {
+  if (auto *Conv = dyn_cast_or_null(Callee))
+if (isRecordWithAttr(Conv->getConversionType()))
+  return true;
+  if (!Callee->getParent()->isInStdNamespace() || !Callee->getIdentifier())
+return false;
+  if (!isRecordWithAttr(Callee->getThisObjectType()) &&
+  !isRecordWithAttr(Callee->getThisObjectType()))
+return false;
+  if (!isRecordWithAttr(Callee->getReturnType()) &&
+  !Callee->getReturnType()->isPointerType())
+return false;
+  return llvm::StringSwitch(Callee->getName())
+  .Cases("begin", "rbegin", "cbegin", "crbegin", true)
+  .Cases("end", "rend", "cend", "crend", true)
+  .Cases("c_str", "data", "get", true)
+  .Default(false);
+}
+
 static void handleGslAnnotatedTypes(IndirectLocalPath , Expr *Call,
 LocalVisitor Visit) {
   auto VisitPointerArg = [&](const Decl *D, Expr *Arg) {
@@ -6577,10 +6596,9 @@
   };
 
   if (auto *MCE = dyn_cast(Call)) {
-const FunctionDecl *Callee = MCE->getDirectCallee();
-if (auto *Conv = dyn_cast_or_null(Callee))
-  if (isRecordWithAttr(Conv->getConversionType()))
-VisitPointerArg(Callee, MCE->getImplicitObjectArgument());
+const auto *MD = cast_or_null(MCE->getDirectCallee());
+if 

[PATCH] D65919: [clang-tidy] Add llvm-prefer-register-over-unsigned check and apply it to LLVM

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp:40
+
+  diag(UserVarDecl->getLocation(), "var %0 is %1 but holds a register")
+  << UserVarDecl << *VarType;

How about `variable %0 declared as %1; use '%2' instead` and move it below to 
where the fix-it is issued?



Comment at: 
clang-tools-extra/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp:44
+  const DeclContext *Context = UserVarDecl->getDeclContext();
+  while((Context = Context->getParent()) != nullptr) {
+if (const auto *Namespace = dyn_cast(Context))

Formatting looks off here, you should run the patch through clang-format.



Comment at: 
clang-tools-extra/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp:53
+  }
+  diag(UserVarDecl->getLocation(), "use '%0'", DiagnosticIDs::Note)
+  << Replacement

I don't think you should issue a second diagnostic on the same line. Instead, 
only issue the previous diagnostic with the fixit attached to it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65919



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

[PATCH] D65819: [Driver][Bundler] Improve bundling of object files.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Additional note. Seems to me, it has something to do with the partial linking. 
According to ld documentation, it is recommended to use `-Ur` option for 
partial linking of the C++ object files to resolve global constructors.

  -Ur
  For anything other than C++ programs, this option is equivalent to '-r': it 
generates relocatable output--i.e., an output file that can in turn serve as 
input to ld. When linking C++ programs, `-Ur' does resolve references to 
constructors, unlike '-r'. It does not work to use '-Ur' on files that were 
themselves linked with `-Ur'; once the constructor table has been built, it 
cannot be added to. Use `-Ur' only for the last partial link, and '-r' for the 
others.

The problem I saw is exactly connected with the global constructors, which are 
not called after partial linking.
Seems to me, we partially link objects files for C++ with `-Ur` option but we 
cannot say if this the last time we perform partial linking or not (we may try 
to bundle/unbundle objects several times, especially when we'll try to support 
linking with libraries). Better not to use partial linking in bundler.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65819



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

[PATCH] D65912: [clang-tidy] Add new check for math constants

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/misc/MathConstantsCheck.cpp:44-58
+{{"M_E"}, {"std::math::e"}, 2.7182818284590452354},
+{{"M_LOG2E"}, {"std::math::log2e"}, 1.4426950408889634074},
+{{"M_LOG10E"}, {"std::math::log10e"}, 0.43429448190325182765},
+{{"M_LN2"}, {"std::math::ln2"}, 0.69314718055994530942},
+{{"M_LN10"}, {"std::math::ln10"}, 2.30258509299404568402},
+{{"M_PI"}, {"std::math::pi"}, 3.14159265358979323846},
+{{"M_PI_2"}, {}, 1.57079632679489661923},

I'm a bit uncomfortable putting the constant values in like this -- are you 
sure those values will be correct for all targets?



Comment at: clang-tools-extra/clang-tidy/misc/MathConstantsCheck.h:26
+
+/// FIXME: Write a short description.
+///

You should fix this FIXME. ;-)



Comment at: clang-tools-extra/clang-tidy/misc/MathConstantsCheck.h:42
+
+  struct MathConstantDescription
+  {

Formatting is off here, you should run the patch through clang-format, if you 
don't already.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65912



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

[PATCH] D65969: [clang][NFC] Consolidating usage of "FinalPhase" in Driver::BuildActions

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 214222.
plotfi added a comment.

adding context


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65969

Files:
  clang/lib/Driver/Driver.cpp

Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3157,17 +3157,6 @@
 return;
   }
 
-  Arg *FinalPhaseArg;
-  phases::ID FinalPhase = getFinalPhase(Args, );
-
-  if (FinalPhase == phases::Link) {
-if (Args.hasArg(options::OPT_emit_llvm))
-  Diag(clang::diag::err_drv_emit_llvm_link);
-if (IsCLMode() && LTOMode != LTOK_None &&
-!Args.getLastArgValue(options::OPT_fuse_ld_EQ).equals_lower("lld"))
-  Diag(clang::diag::err_drv_lto_without_lld);
-  }
-
   // Reject -Z* at the top level, these options should never have been exposed
   // by gcc.
   if (Arg *A = Args.getLastArg(options::OPT_Z_Joined))
@@ -3220,7 +3209,29 @@
 Args.eraseArg(options::OPT__SLASH_Yc);
 YcArg = nullptr;
   }
-  if (FinalPhase == phases::Preprocess || Args.hasArg(options::OPT__SLASH_Y_)) {
+
+  // Builder to be used to build offloading actions.
+  OffloadingActionBuilder OffloadBuilder(C, Args, Inputs);
+
+  // Construct the actions to perform.
+  HeaderModulePrecompileJobAction *HeaderModuleAction = nullptr;
+  ActionList LinkerInputs;
+
+  phases::ID FinalPhase;
+  {
+Arg *FinalPhaseArg;
+FinalPhase = getFinalPhase(Args, );
+
+if (FinalPhase == phases::Link) {
+  if (Args.hasArg(options::OPT_emit_llvm))
+Diag(clang::diag::err_drv_emit_llvm_link);
+  if (IsCLMode() && LTOMode != LTOK_None &&
+  !Args.getLastArgValue(options::OPT_fuse_ld_EQ).equals_lower("lld"))
+Diag(clang::diag::err_drv_lto_without_lld);
+}
+
+if (FinalPhase == phases::Preprocess ||
+Args.hasArg(options::OPT__SLASH_Y_)) {
   // If only preprocessing or /Y- is used, all pch handling is disabled.
   // Rather than check for it everywhere, just remove clang-cl pch-related
   // flags here.
@@ -3230,13 +3241,6 @@
   YcArg = YuArg = nullptr;
 }
 
-  // Builder to be used to build offloading actions.
-  OffloadingActionBuilder OffloadBuilder(C, Args, Inputs);
-
-  // Construct the actions to perform.
-  HeaderModulePrecompileJobAction *HeaderModuleAction = nullptr;
-  ActionList LinkerInputs;
-
 unsigned LastPLSize = 0;
 for (auto  : Inputs) {
   types::ID InputType = I.first;
@@ -3268,7 +3272,10 @@
 // Special case '-E' warning on a previously preprocessed file to make
 // more sense.
 else if (InitialPhase == phases::Compile &&
-   FinalPhase == phases::Preprocess &&
+ (Args.getLastArg(options::OPT__SLASH_EP,
+  options::OPT__SLASH_P) ||
+  Args.getLastArg(options::OPT_E) ||
+  Args.getLastArg(options::OPT_M, options::OPT_MM)) &&
  getPreprocessedType(InputType) == types::TY_INVALID)
   Diag(clang::diag::warn_drv_preprocessed_input_file_unused)
   << InputArg->getAsString(Args) << !!FinalPhaseArg
@@ -3288,8 +3295,7 @@
   llvm::SmallVector PCHPL;
   types::getCompilationPhases(HeaderType, PCHPL);
   // Build the pipeline for the pch file.
-Action *ClangClPch =
-C.MakeAction(*InputArg, HeaderType);
+  Action *ClangClPch = C.MakeAction(*InputArg, HeaderType);
   for (phases::ID Phase : PCHPL)
 ClangClPch = ConstructPhaseAction(C, Args, Phase, ClangClPch);
   assert(ClangClPch);
@@ -3301,6 +3307,25 @@
   // probably not be considered successful either.
 }
   }
+}
+
+// If we are linking, claim any options which are obviously only used for
+// compilation.
+// FIXME: Understand why the last Phase List length is used here.
+if (FinalPhase == phases::Link && LastPLSize == 1) {
+  Args.ClaimAllArgs(options::OPT_CompileOnly_Group);
+  Args.ClaimAllArgs(options::OPT_cl_compile_Group);
+}
+  }
+
+  for (auto  : Inputs) {
+types::ID InputType = I.first;
+const Arg *InputArg = I.second;
+
+llvm::SmallVector PL;
+types::getCompilationPhases(InputType, PL);
+if (PL[0] > FinalPhase)
+  continue;
 
 // Build the pipeline for this file.
 Action *Current = C.MakeAction(*InputArg, InputType);
@@ -3379,14 +3404,6 @@
 Actions.push_back(LA);
   }
 
-  // If we are linking, claim any options which are obviously only used for
-  // compilation.
-  // FIXME: Understand why the last Phase List length is used here.
-  if (FinalPhase == phases::Link && LastPLSize == 1) {
-Args.ClaimAllArgs(options::OPT_CompileOnly_Group);
-Args.ClaimAllArgs(options::OPT_cl_compile_Group);
-  }
-
   // If --print-supported-cpus, -mcpu=? or -mtune=? is specified, build a custom
   // Compile 

[PATCH] D65723: [analyzer][NFC] Add different interestingness kinds

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

As this thing will now be part of the checker interface it would be great to 
have some guidelines which interestingness kind to use (or, when to use 
interestingness at all). I am totally fine with addressing this in a followup 
patch though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65723



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

[PATCH] D65575: [analyzer] Mention whether an event is about a condition in a bug report part 1

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

Yeah, this is important bike-shedding from the user point of view. Otherwise it 
looks good to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65575



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

[PATCH] D65969: [clang][NFC] Consolidating usage of "FinalPhase" in Driver::BuildActions

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi created this revision.
plotfi added reviewers: aaron.ballman, compnerd.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

I am working to remove this concept of the "FinalPhase" in the clang driver, 
but it is used in a lot of different places to do argument handling for 
different combinatios of phase pipelines and arguments. I am trying to 
consolidate most of the uses of "FinalPhase" into its own separate scope.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65969

Files:
  clang/lib/Driver/Driver.cpp

Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3157,16 +3157,6 @@
 return;
   }
 
-  Arg *FinalPhaseArg;
-  phases::ID FinalPhase = getFinalPhase(Args, );
-
-  if (FinalPhase == phases::Link) {
-if (Args.hasArg(options::OPT_emit_llvm))
-  Diag(clang::diag::err_drv_emit_llvm_link);
-if (IsCLMode() && LTOMode != LTOK_None &&
-!Args.getLastArgValue(options::OPT_fuse_ld_EQ).equals_lower("lld"))
-  Diag(clang::diag::err_drv_lto_without_lld);
-  }
 
   // Reject -Z* at the top level, these options should never have been exposed
   // by gcc.
@@ -3220,7 +3210,29 @@
 Args.eraseArg(options::OPT__SLASH_Yc);
 YcArg = nullptr;
   }
-  if (FinalPhase == phases::Preprocess || Args.hasArg(options::OPT__SLASH_Y_)) {
+
+  // Builder to be used to build offloading actions.
+  OffloadingActionBuilder OffloadBuilder(C, Args, Inputs);
+
+  // Construct the actions to perform.
+  HeaderModulePrecompileJobAction *HeaderModuleAction = nullptr;
+  ActionList LinkerInputs;
+
+  phases::ID FinalPhase;
+  {
+Arg *FinalPhaseArg;
+FinalPhase = getFinalPhase(Args, );
+
+if (FinalPhase == phases::Link) {
+  if (Args.hasArg(options::OPT_emit_llvm))
+Diag(clang::diag::err_drv_emit_llvm_link);
+  if (IsCLMode() && LTOMode != LTOK_None &&
+  !Args.getLastArgValue(options::OPT_fuse_ld_EQ).equals_lower("lld"))
+Diag(clang::diag::err_drv_lto_without_lld);
+}
+
+if (FinalPhase == phases::Preprocess ||
+Args.hasArg(options::OPT__SLASH_Y_)) {
   // If only preprocessing or /Y- is used, all pch handling is disabled.
   // Rather than check for it everywhere, just remove clang-cl pch-related
   // flags here.
@@ -3230,13 +3242,6 @@
   YcArg = YuArg = nullptr;
 }
 
-  // Builder to be used to build offloading actions.
-  OffloadingActionBuilder OffloadBuilder(C, Args, Inputs);
-
-  // Construct the actions to perform.
-  HeaderModulePrecompileJobAction *HeaderModuleAction = nullptr;
-  ActionList LinkerInputs;
-
 unsigned LastPLSize = 0;
 for (auto  : Inputs) {
   types::ID InputType = I.first;
@@ -3268,7 +3273,10 @@
 // Special case '-E' warning on a previously preprocessed file to make
 // more sense.
 else if (InitialPhase == phases::Compile &&
-   FinalPhase == phases::Preprocess &&
+ (Args.getLastArg(options::OPT__SLASH_EP,
+  options::OPT__SLASH_P) ||
+  Args.getLastArg(options::OPT_E) ||
+  Args.getLastArg(options::OPT_M, options::OPT_MM)) &&
  getPreprocessedType(InputType) == types::TY_INVALID)
   Diag(clang::diag::warn_drv_preprocessed_input_file_unused)
   << InputArg->getAsString(Args) << !!FinalPhaseArg
@@ -3288,8 +3296,7 @@
   llvm::SmallVector PCHPL;
   types::getCompilationPhases(HeaderType, PCHPL);
   // Build the pipeline for the pch file.
-Action *ClangClPch =
-C.MakeAction(*InputArg, HeaderType);
+  Action *ClangClPch = C.MakeAction(*InputArg, HeaderType);
   for (phases::ID Phase : PCHPL)
 ClangClPch = ConstructPhaseAction(C, Args, Phase, ClangClPch);
   assert(ClangClPch);
@@ -3301,6 +3308,25 @@
   // probably not be considered successful either.
 }
   }
+}
+
+// If we are linking, claim any options which are obviously only used for
+// compilation.
+// FIXME: Understand why the last Phase List length is used here.
+if (FinalPhase == phases::Link && LastPLSize == 1) {
+  Args.ClaimAllArgs(options::OPT_CompileOnly_Group);
+  Args.ClaimAllArgs(options::OPT_cl_compile_Group);
+}
+  }
+
+  for (auto  : Inputs) {
+types::ID InputType = I.first;
+const Arg *InputArg = I.second;
+
+llvm::SmallVector PL;
+types::getCompilationPhases(InputType, PL);
+if (PL[0] > FinalPhase)
+  continue;
 
 // Build the pipeline for this file.
 Action *Current = C.MakeAction(*InputArg, InputType);
@@ -3379,14 +3405,6 @@
 Actions.push_back(LA);
   }
 
-  // If we are linking, claim any options which are obviously only used for
-  // compilation.
-  // FIXME: Understand why the last Phase List length is used here.
-  if (FinalPhase == 

[PATCH] D65287: [analyzer][CFG] Don't track the condition of asserts

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: clang/include/clang/Analysis/CFG.h:859
+  /// Returns true if the block would eventually end with a sink (a noreturn
+  /// node). We scan the child CFG blocks in a depth-first manner and see if
+  /// all paths eventually end up in an immediate sink block.

I think the depth-first manner is an implementation detail and the result of 
this method should not depend on the traversal strategy. I would remove that 
part from the comment.



Comment at: clang/lib/Analysis/CFG.cpp:5634
+  // immediately caught.
+  if (std::any_of(Blk->begin(), Blk->end(), [](const CFGElement ) {
+if (Optional StmtElm = Elm.getAs())

I am wondering, should we actually scan the whole `CFGBlock` for `ThrowExpr`s? 
Shouldn't `Throw` be the terminator? (In case we can get that easily) In case 
we are afraid of seeing lifetime end or other blocks AFTER throw, maybe it is 
more efficient to start scanning from the last element ad early return on the 
first non-throw stmt?



Comment at: clang/lib/Analysis/CFG.cpp:5649
+  const CFGBlock *StartBlk = this;
+  if (!StartBlk)
+return false;

This should never be null, I think this should be dead code.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1750
+  //   [B1] -> [B2] -> [B3] -> [sink]
+  // assert(A && B || C);\   \   \
+  //  ---> [go on with the execution]

baloghadamsoftware wrote:
> xazax.hun wrote:
> > I wonder if the CFG is correct for your example. If A evaluates to false, 
> > we still check C before the sink. If A evaluates to true we still check B 
> > before going on. But maybe it is just late for me :)
> I think an arrow from `[B1]` to `[B3]` is missing for the `A == false` case.
I am still not sure I like this picture. Looking at [B1] I had the impression 
it has 3 successors which is clearly not the case. 


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

https://reviews.llvm.org/D65287



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

r368338 - [analyzer] Fix scan-build's plist output in plist-html mode.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Thu Aug  8 13:22:32 2019
New Revision: 368338

URL: http://llvm.org/viewvc/llvm-project?rev=368338=rev
Log:
[analyzer] Fix scan-build's plist output in plist-html mode.

r366941 accidentally made it delete all plist files
as soon as they're produced.

Modified:
cfe/trunk/tools/scan-build/libexec/ccc-analyzer

Modified: cfe/trunk/tools/scan-build/libexec/ccc-analyzer
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build/libexec/ccc-analyzer?rev=368338=368337=368338=diff
==
--- cfe/trunk/tools/scan-build/libexec/ccc-analyzer (original)
+++ cfe/trunk/tools/scan-build/libexec/ccc-analyzer Thu Aug  8 13:22:32 2019
@@ -118,7 +118,7 @@ my $ResultFile;
 
 # Remove any stale files at exit.
 END {
-  if (defined $ResultFile && $ResultFile ne "") {
+  if (defined $ResultFile && -z $ResultFile) {
 unlink($ResultFile);
   }
   if (defined $CleanupFile) {


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

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:94
+  [CastFromName, CastToName, IsNullReturn,
+   IsSimpleCast](BugReport &) -> std::string {
 SmallString<128> Msg;

`IsDynamicCast`.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:98
 
-Out << "Assuming dynamic cast from '" << CastFromName << "' to '"
-<< CastToName << "' succeeds";
-return Out.str();
-  },
-  /*IsPrunable=*/true);
+Out << (!IsSimpleCast ? "Assuming dynamic cast " : "Dynamic cast ");
+if (CastFromName)

More suggestions for the `:`-branch: "Hard cast" (a bit too jargon-y), "Safe 
cast", "Checked cast".



Comment at: clang/test/Analysis/cast-value.cpp:156-167
+void evalNonNullParamNonNullReturn(const Shape *S) {
+  const auto *C = cast(S);
+  // expected-note@-1 {{Dynamic cast from 'Shape' to 'Circle' succeeds}}
+  // expected-note@-2 {{Assuming pointer value is null}}
+  // expected-note@-3 {{'C' initialized here}}
+
+  (void)(1 / !(bool)C);

Mmm, wait a sec. That's a false positive. `cast<>` doesn't accept null 
pointers. We have `cast_or_null` for this.


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

https://reviews.llvm.org/D65889



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

r368334 - [clang] add REQUIRES: linux to driver test case

[Brian Cain via cfe-commits]

Author: bcain
Date: Thu Aug  8 13:12:54 2019
New Revision: 368334

URL: http://llvm.org/viewvc/llvm-project?rev=368334=rev
Log:
[clang] add REQUIRES: linux to driver test case

The test case explicitly leverages linux, so should include it as
a test requirement.

Modified:
cfe/trunk/test/Driver/as-no-warnings.c

Modified: cfe/trunk/test/Driver/as-no-warnings.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/as-no-warnings.c?rev=368334=368333=368334=diff
==
--- cfe/trunk/test/Driver/as-no-warnings.c (original)
+++ cfe/trunk/test/Driver/as-no-warnings.c Thu Aug  8 13:12:54 2019
@@ -7,6 +7,7 @@
 
 // REQUIRES: clang-driver
 // REQUIRES: x86-registered-target
+// REQUIRES: linux
 
 // CHECK: "-cc1" {{.*}} "-massembler-no-warn"
 // CHECK-NOIAS: "--no-warn"


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

[PATCH] D65906: [clang-scan-deps] Fix edge cases in the minimizer

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

dexonsmith added a comment.

In D65906#1621542 , @aganea wrote:

> In D65906#1620034 , @arphaman wrote:
>
> > Regarding the invisible characters before `#ifdef`, are you sure that's the 
> > right behavior?
>
>
> Should we then copy these invisible characters to the minimized output? 
> Believe it or not, these characters are there in our codebase since 2013, and 
> never clang has complained about it :)


Do you have an example command-line where clang doesn't error with your test 
file?  Alex gave you an example where it does error.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65906



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

r368332 - [clang] add REQUIRES to driver test case

[Brian Cain via cfe-commits]

Author: bcain
Date: Thu Aug  8 13:04:39 2019
New Revision: 368332

URL: http://llvm.org/viewvc/llvm-project?rev=368332=rev
Log:
[clang] add REQUIRES to driver test case

The test case explicitly leverages x86, so should include it as
a test requirement.

Modified:
cfe/trunk/test/Driver/as-no-warnings.c

Modified: cfe/trunk/test/Driver/as-no-warnings.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/as-no-warnings.c?rev=368332=368331=368332=diff
==
--- cfe/trunk/test/Driver/as-no-warnings.c (original)
+++ cfe/trunk/test/Driver/as-no-warnings.c Thu Aug  8 13:04:39 2019
@@ -5,6 +5,9 @@
 // RUN: not %clang %s -c -o %t.o -target i686-pc-linux-gnu -integrated-as 
-Wa,--fatal-warnings 2>&1 | FileCheck --check-prefix=CHECK-AS-FATAL %s
 // RUN: not %clang %s -c -o %t.o -target i686-pc-linux-gnu -fno-integrated-as 
-Wa,--fatal-warnings 2>&1 | FileCheck --check-prefix=CHECK-AS-FATAL %s
 
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+
 // CHECK: "-cc1" {{.*}} "-massembler-no-warn"
 // CHECK-NOIAS: "--no-warn"
 // CHECK-AS-NOWARN-NOT: warning:


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

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Looking good now! I still recommend eventually adding tests with casts of 
references.

In D65889#1620128 , @Charusso wrote:

> - May it is better to also check for `getAsCXXRecordDecl()` for obtaining a 
> class.




In D65889#1621587 , @Charusso wrote:

> - The call as `getAsCXXRecordDecl()` sometimes run into an assertion failure, 
> so we need to avoid it


Well, yeah, i was just about to say "good thing that you don't do this because 
we really don't want to get into business of modeling return-by-value `getAs` 
calls such as `SVal::getAs`".




Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:51
+
+  const CallDescriptionMap SugarCastCDM = {
+  {{{"clang", "castAs"}, 0}, ::evalCastAs},

Charusso wrote:
> NoQ wrote:
> > I'd rather have only one map from call descriptions to (callback, kind) 
> > pairs. This should make the `evalCall` code much more readable.
> The problem with that I really want to use something like that:
> ```
> auto &&[Check, Kind, MoreData] = *Lookup;
> ```
> but I cannot. Until that time it equally non-readable and difficult to scale 
> sadly. For now it is fine, but that C++17 feature would be more awesome.
I think it turned out pretty pretty (no pun intended).



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:26
 class CastValueChecker : public Checker {
+  enum class CastKind { Checking, Sugar };
+

Please explain "Checking" and "Sugar". Checking what? Sugar... you mean 
syntactic sugar? For what?



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:220
 
-  // If we cannot obtain both of the classes we cannot be sure how to model it.
-  if (!CE->getType()->getPointeeCXXRecordDecl() ||
-  !CE->getArg(0)->getType()->getPointeeCXXRecordDecl())
-return false;
+  const CastCheck *Check = >first;
+  CastKind Kind = Lookup->second;

What's the point of making this a raw pointer? (no pun intended)


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

https://reviews.llvm.org/D65889



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

[PATCH] D65917: [clang-tidy] Added check for the Google style guide's category method naming rule.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp:44
+CheckFactories.registerCheck(
+"google-objc-require-category-method-prefixes");
 CheckFactories.registerCheck(

Please keep this list in alphabetical order.



Comment at: 
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:21
+  // This check should be run only on Objective-C code.
+  if (!getLangOpts().ObjC) {
+return;

You should elide the braces here per our usual coding conventions (also applies 
elsewhere).



Comment at: 
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:25
+
+  auto method_declaration =
+  objcMethodDecl().bind(kCustomCategoryMethodIdentifier);

No need for the local here, I'd just inline it below.



Comment at: 
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:37
+RequireCategoryMethodPrefixesCheck::matching_whitelisted_prefix(
+std::string class_name) {
+  std::vector prefix_array =

I think this interface should accept a `StringRef` argument and either return a 
`std::string` or an `llvm::Optional` rather than allocating a 
pointer.



Comment at: 
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:43
+const char *prefix = iterator.c_str();
+if (!strncmp(class_name.c_str(), prefix, strlen(prefix))) {
+  return llvm::make_unique(prefix);

No need for messy C APIs here -- you can compare a `StringRef` to a 
`std::string` directly.



Comment at: 
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:57
+  }
+  std::string method_name = method_declaration->getNameAsString();
+  auto owning_objc_class_interface = method_declaration->getClassInterface();

This should use `getName()` to get a `StringRef` to avoid the copy.



Comment at: 
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:62
+  }
+  std::string class_name = owning_objc_class_interface->getNameAsString();
+

Can use `getName()` and assign to a `StringRef`



Comment at: 
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:100
+  diag(method_declaration->getBeginLoc(),
+   "the category method %0 is not properly prefixed.")
+  << method_name;

Drop the period at the end of the diagnostic -- clang-tidy diagnostics are not 
meant to be grammatically correct.



Comment at: 
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:101
+   "the category method %0 is not properly prefixed.")
+  << method_name;
+}

You should pass `method_declaration` instead; it will get quoted and converted 
to the proper identifier automatically.



Comment at: 
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.h:28
+  /// Defaults to empty.
+  const std::string whitelisted_prefixes_;
+

You should pick names that match the LLVM coding style naming rules (here and 
elsewhere in the patch).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65917



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

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

[Jian Cai via Phabricator via cfe-commits]

jcai19 added a comment.

In D65019#1621670 , @efriedma wrote:

> Yes, it's technically a "call", but you don't need the call lowering code.  
> That's dedicated to stuff like passing arguments, returning values, checking 
> whether the function can be tail-called, etc.  None of that applies here; the 
> intrinsic always corresponds to exactly one pseudo-instruction, a BL_PUSHLR.


Thanks for the clarification. I will look into it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65019



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

[PATCH] D65127: Even more warnings utilizing gsl::Owner/gsl::Pointer annotations

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun marked an inline comment as done.
xazax.hun added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:6581
+if (!Callee->getIdentifier()) {
+  auto OO = Callee->getOverloadedOperator();
+  return OO == OverloadedOperatorKind::OO_Subscript ||

xazax.hun wrote:
> gribozavr wrote:
> > xazax.hun wrote:
> > > gribozavr wrote:
> > > > mgehre wrote:
> > > > > xazax.hun wrote:
> > > > > > If we want to relax the warnings to give more results we could 
> > > > > > extend the checking of these overloaded operators for annotated 
> > > > > > types. But this would imply that the user need to have the expected 
> > > > > > semantics for those types and can only suppress false positives by 
> > > > > > removing some gsl:::owner/poinnter annotations.
> > > > > I see those options:
> > > > > - Either gsl::Owner implies some specific form of those operators 
> > > > > (and if that does not hold for a class, then one should not annotate 
> > > > > it with gsl::Owner)
> > > > > - or gsl::Owner only implies some specific behavior for the 
> > > > > "gsl::Pointer constructed from gsl::Owner" case and everything else 
> > > > > requires additional annotation
> > > > > I expect that we will experiment a bit in the future to see what 
> > > > > works well for real-world code.
> > > > I understand the difficulty, but I don't think it is appropriate to 
> > > > experiment by ourselves -- these attributes are defined in a spec, and 
> > > > if something is not clear, the spec should be clarified.
> > > This is exactly what is going to happen but I think it would be 
> > > unfortunate to stall the progress until the new version of the spec 
> > > materializes. The idea is to keep the implementations and the specs in 
> > > sync, but as Herb has other projects too, it takes some time to channel 
> > > the experience back into the spec. As the current version of the warnings 
> > > found true positives in real world projects and we have yet to see any 
> > > false positives I would prefer to move forward to maximize utility.  
> > > 
> > > 
> > I don't understand how different implementations can ever converge in that 
> > case.
> > 
> > If this language extension is not sufficiently designed yet, maybe it is 
> > not ready for inclusion in Clang?
> > 
> > 
> The MSVC implementation does not support user defined annotations yet, so we 
> are the first one to ask such questions like is it valid for an user to 
> annotate a type as gsl::Pointer and have an overloaded deref operator with 
> functionality other than accessing the pointee. We already forwarded these 
> concerns to Herb, and he promised to clarify these things in the paper. Once 
> it is clarified, MSVC will also follow it. Since this code will not reach the 
> wider audience until Clang 10 is released and it is pretty easy to change 
> this detail I do not see the justification to postpone the inclusion.
> 
> If we postpone the inclusion over and over we will never get enough 
> experience from real world users to ever have enough confidence. 
Ok, after discussing this with Herb the conclusion is the following. The paper 
does not have any requirements for any of the methods for Pointers or Owners. 
If a method has a semantics that is not a good match with the default rules the 
user can annotate the methods. As method annotations are coming later, the 
right approach is to only rely on the semantics of these operators for STL 
types for now, exactly what is implemented in this patch.

And again, just to make it clear, the reason why we want to add this first 
rather than the annotations because the latter is a rather large patch and we 
want to gain more real world experience first before committing to a specific 
approach. Yet, all of the true positives we found so far needs these 
assumptions about STL types, so it really is useful to have this until we add 
the support for annotations. 


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

https://reviews.llvm.org/D65127



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

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Yes, it's technically a "call", but you don't need the call lowering code.  
That's dedicated to stuff like passing arguments, returning values, checking 
whether the function can be tail-called, etc.  None of that applies here; the 
intrinsic always corresponds to exactly one pseudo-instruction, a BL_PUSHLR.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65019



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

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM




Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:820
+  llvm::DenseMap>
+  CompatibleAliases;

gribozavr wrote:
> `unordered_set`?
or `SmallPtrSet`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60543



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

r368328 - [clang] Add no-warn support for Wa

[Brian Cain via cfe-commits]

Author: bcain
Date: Thu Aug  8 12:19:20 2019
New Revision: 368328

URL: http://llvm.org/viewvc/llvm-project?rev=368328=rev
Log:
[clang] Add no-warn support for Wa

Added:
cfe/trunk/test/Driver/as-no-warnings.c
Modified:
cfe/trunk/include/clang/Basic/CodeGenOptions.def
cfe/trunk/include/clang/Driver/CC1Options.td
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/tools/driver/cc1as_main.cpp

Modified: cfe/trunk/include/clang/Basic/CodeGenOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/CodeGenOptions.def?rev=368328=368327=368328=diff
==
--- cfe/trunk/include/clang/Basic/CodeGenOptions.def (original)
+++ cfe/trunk/include/clang/Basic/CodeGenOptions.def Thu Aug  8 12:19:20 2019
@@ -133,6 +133,7 @@ CODEGENOPT(NoDwarfDirectoryAsm , 1, 0) /
 CODEGENOPT(NoExecStack   , 1, 0) ///< Set when -Wa,--noexecstack is 
enabled.
 CODEGENOPT(FatalWarnings , 1, 0) ///< Set when -Wa,--fatal-warnings is
  ///< enabled.
+CODEGENOPT(NoWarn, 1, 0) ///< Set when -Wa,--no-warn is enabled.
 CODEGENOPT(EnableSegmentedStacks , 1, 0) ///< Set when -fsplit-stack is 
enabled.
 CODEGENOPT(NoImplicitFloat   , 1, 0) ///< Set when -mno-implicit-float is 
enabled.
 CODEGENOPT(NoInfsFPMath  , 1, 0) ///< Assume FP arguments, results not 
+-Inf.

Modified: cfe/trunk/include/clang/Driver/CC1Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=368328=368327=368328=diff
==
--- cfe/trunk/include/clang/Driver/CC1Options.td (original)
+++ cfe/trunk/include/clang/Driver/CC1Options.td Thu Aug  8 12:19:20 2019
@@ -200,6 +200,8 @@ def compress_debug_sections_EQ : Joined<
 HelpText<"DWARF debug sections compression type">;
 def mno_exec_stack : Flag<["-"], "mnoexecstack">,
   HelpText<"Mark the file as not needing an executable stack">;
+def massembler_no_warn : Flag<["-"], "massembler-no-warn">,
+  HelpText<"Make assembler not emit warnings">;
 def massembler_fatal_warnings : Flag<["-"], "massembler-fatal-warnings">,
   HelpText<"Make assembler warnings fatal">;
 def mrelax_relocations : Flag<["--"], "mrelax-relocations">,

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=368328=368327=368328=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Thu Aug  8 12:19:20 2019
@@ -493,6 +493,7 @@ static void initTargetOptions(llvm::Targ
   CodeGenOpts.IncrementalLinkerCompatible;
   Options.MCOptions.MCPIECopyRelocations = CodeGenOpts.PIECopyRelocations;
   Options.MCOptions.MCFatalWarnings = CodeGenOpts.FatalWarnings;
+  Options.MCOptions.MCNoWarn = CodeGenOpts.NoWarn;
   Options.MCOptions.AsmVerbose = CodeGenOpts.AsmVerbose;
   Options.MCOptions.PreserveAsmComments = CodeGenOpts.PreserveAsmComments;
   Options.MCOptions.ABIName = TargetOpts.ABI;

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=368328=368327=368328=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Thu Aug  8 12:19:20 2019
@@ -2169,6 +2169,8 @@ static void CollectArgsForIntegratedAsse
 CmdArgs.push_back("-msave-temp-labels");
   } else if (Value == "--fatal-warnings") {
 CmdArgs.push_back("-massembler-fatal-warnings");
+  } else if (Value == "--no-warn") {
+CmdArgs.push_back("-massembler-no-warn");
   } else if (Value == "--noexecstack") {
 UseNoExecStack = true;
   } else if (Value.startswith("-compress-debug-sections") ||

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=368328=368327=368328=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Thu Aug  8 12:19:20 2019
@@ -888,6 +888,7 @@ static bool ParseCodeGenArgs(CodeGenOpti
   Opts.NumRegisterParameters = getLastArgIntValue(Args, OPT_mregparm, 0, 
Diags);
   Opts.NoExecStack = Args.hasArg(OPT_mno_exec_stack);
   Opts.FatalWarnings = Args.hasArg(OPT_massembler_fatal_warnings);
+  Opts.NoWarn = Args.hasArg(OPT_massembler_no_warn);
   Opts.EnableSegmentedStacks = Args.hasArg(OPT_split_stacks);
   Opts.RelaxAll = Args.hasArg(OPT_mrelax_all);
   Opts.IncrementalLinkerCompatible =

Added: 

r368327 - [llvm-mc] Add reportWarning() to MCContext

[Brian Cain via cfe-commits]

Author: bcain
Date: Thu Aug  8 12:13:23 2019
New Revision: 368327

URL: http://llvm.org/viewvc/llvm-project?rev=368327=rev
Log:
[llvm-mc] Add reportWarning() to MCContext

Adding reportWarning() to MCContext, so that it can be used from
the Hexagon assembler backend.

Modified:
cfe/trunk/tools/driver/cc1as_main.cpp

Modified: cfe/trunk/tools/driver/cc1as_main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/driver/cc1as_main.cpp?rev=368327=368326=368327=diff
==
--- cfe/trunk/tools/driver/cc1as_main.cpp (original)
+++ cfe/trunk/tools/driver/cc1as_main.cpp Thu Aug  8 12:13:23 2019
@@ -374,7 +374,8 @@ static bool ExecuteAssembler(AssemblerIn
   // MCObjectFileInfo needs a MCContext reference in order to initialize 
itself.
   std::unique_ptr MOFI(new MCObjectFileInfo());
 
-  MCContext Ctx(MAI.get(), MRI.get(), MOFI.get(), );
+  MCTargetOptions MCOptions;
+  MCContext Ctx(MAI.get(), MRI.get(), MOFI.get(), , );
 
   bool PIC = false;
   if (Opts.RelocationModel == "static") {
@@ -431,7 +432,6 @@ static bool ExecuteAssembler(AssemblerIn
   raw_pwrite_stream *Out = FDOS.get();
   std::unique_ptr BOS;
 
-  MCTargetOptions MCOptions;
   MCOptions.ABIName = Opts.TargetABI;
 
   // FIXME: There is a bit of code duplication with addPassesToEmitFile.


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

[PATCH] D65863: [ARM] Add support for the s,j,x,N,O inline asm constraints

[David Candler via Phabricator via cfe-commits]

dcandler updated this revision to Diff 214205.
dcandler marked an inline comment as done.
dcandler added a comment.

Adjusted the formatting on some comment lines, and added FIXMEs for all the 
constraints that require additional validation to clarify what is still needed 
and where.


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

https://reviews.llvm.org/D65863

Files:
  clang/lib/Basic/Targets/ARM.cpp
  clang/test/Sema/arm_inline_asm_constraints.c
  llvm/lib/Target/ARM/ARMISelLowering.cpp

Index: llvm/lib/Target/ARM/ARMISelLowering.cpp
===
--- llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -15143,7 +15143,7 @@
   case 'j':
 // Constant suitable for movw, must be between 0 and
 // 65535.
-if (Subtarget->hasV6T2Ops())
+if (Subtarget->hasV6T2Ops() || (Subtarget->hasV8MBaselineOps()))
   if (CVal >= 0 && CVal <= 65535)
 break;
 return;
@@ -15251,7 +15251,7 @@
 return;
 
   case 'N':
-if (Subtarget->isThumb()) {  // FIXME thumb2
+if (Subtarget->isThumb1Only()) {
   // This must be a constant between 0 and 31, for shift amounts.
   if (CVal >= 0 && CVal <= 31)
 break;
@@ -15259,7 +15259,7 @@
 return;
 
   case 'O':
-if (Subtarget->isThumb()) {  // FIXME thumb2
+if (Subtarget->isThumb1Only()) {
   // This must be a multiple of 4 between -508 and 508, for
   // ADD/SUB sp = sp + immediate.
   if ((CVal >= -508 && CVal <= 508) && ((CVal & 3) == 0))
Index: clang/test/Sema/arm_inline_asm_constraints.c
===
--- /dev/null
+++ clang/test/Sema/arm_inline_asm_constraints.c
@@ -0,0 +1,305 @@
+// REQUIRES: arm-registered-target
+
+// RUN: %clang_cc1 -triple armv6 -verify=arm6 %s
+// RUN: %clang_cc1 -triple armv7 -verify=arm7 %s
+// RUN: %clang_cc1 -triple thumbv6 -verify=thumb1 %s
+// RUN: %clang_cc1 -triple thumbv7 -verify=thumb2 %s
+
+// j: An immediate integer between 0 and 65535 (valid for MOVW) (ARM/Thumb2)
+int test_j(int i) {
+  int res;
+  __asm("movw %0, %1;"
+: [ result ] "=r"(res)
+: [ constant ] "j"(-1), [ input ] "r"(i)
+:);
+  // arm6-error@13 {{invalid input constraint 'j' in asm}}
+  // arm7-error@13 {{value '-1' out of range for constraint 'j'}}
+  // thumb1-error@13 {{invalid input constraint 'j' in asm}}
+  // thumb2-error@13 {{value '-1' out of range for constraint 'j'}}
+  __asm("movw %0, %1;"
+: [ result ] "=r"(res)
+: [ constant ] "j"(0), [ input ] "r"(i)
+:);
+  // arm6-error@21 {{invalid input constraint 'j' in asm}}
+  // arm7-no-error
+  // thumb1-error@21 {{invalid input constraint 'j' in asm}}
+  // thumb2-no-error
+  __asm("movw %0, %1;"
+: [ result ] "=r"(res)
+: [ constant ] "j"(65535), [ input ] "r"(i)
+:);
+  // arm6-error@29 {{invalid input constraint 'j' in asm}}
+  // arm7-no-error
+  // thumb1-error@29 {{invalid input constraint 'j' in asm}}
+  // thumb2-no-error
+  __asm("movw %0, %1;"
+: [ result ] "=r"(res)
+: [ constant ] "j"(65536), [ input ] "r"(i)
+:);
+  // arm6-error@37 {{invalid input constraint 'j' in asm}}
+  // arm7-error@37 {{value '65536' out of range for constraint 'j'}}
+  // thumb1-error@37 {{invalid input constraint 'j' in asm}}
+  // thumb2-error@37 {{value '65536' out of range for constraint 'j'}}
+  return res;
+}
+
+// I: An immediate integer valid for a data-processing instruction. (ARM/Thumb2)
+//An immediate integer between -255 and -1. (Thumb1)
+int test_I(int i) {
+  int res;
+  __asm(
+  "add %0, %1;"
+  : [ result ] "=r"(res)
+  : [ constant ] "I"(-1), [ input ] "r"(i)
+  :); // thumb1-error@53 {{value '-1' out of range for constraint 'I'}}
+  __asm(
+  "add %0, %1;"
+  : [ result ] "=r"(res)
+  : [ constant ] "I"(0), [ input ] "r"(i)
+  :); // No errors expected.
+  __asm(
+  "add %0, %1;"
+  : [ result ] "=r"(res)
+  : [ constant ] "I"(255), [ input ] "r"(i)
+  :); // No errors expected.
+  __asm(
+  "add %0, %1;"
+  : [ result ] "=r"(res)
+  : [ constant ] "I"(256), [ input ] "r"(i)
+  :); // thumb1-error@68 {{value '256' out of range for constraint 'I'}}
+  return res;
+}
+
+// J: An immediate integer between -4095 and 4095. (ARM/Thumb2)
+//An immediate integer between -255 and -1. (Thumb1)
+int test_J(int i) {
+  int res;
+  __asm(
+  "movw %0, %1;"
+  : [ result ] "=r"(res)
+  : [ constant ] "J"(-4096), [ input ] "r"(i)
+  :);
+  // arm6-error@80 {{value '-4096' out of range for constraint 'J'}}
+  // arm7-error@80 {{value '-4096' out of range for constraint 'J'}}
+  // thumb1-error@80 {{value '-4096' out of range for constraint 'J'}}
+  // thumb2-error@80 {{value '-4096' out of range for constraint 'J'}}
+  __asm(

[PATCH] D65863: [ARM] Add support for the s,j,x,N,O inline asm constraints

[David Candler via Phabricator via cfe-commits]

dcandler marked 5 inline comments as done.
dcandler added inline comments.



Comment at: clang/lib/Basic/Targets/ARM.cpp:938
+// Thumb1: An immediate integer which is a multiple of 4 between 0 and 
1020.
+Info.setRequiresImmediate();
 return true;

compnerd wrote:
> Can we leave this as a FIXME?  This needs additional validation on the input.
I think it's not just the `M` constraint that requires additional validation. 
Most of these immediate constraints require values that can fit in specific 
encodings to be valid, or have properties like being a multiple of a number, 
but `setRequiresImmediate` at present can only check against a min/max or exact 
values.


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

https://reviews.llvm.org/D65863



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

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a reviewer: reames.
lebedev.ri added inline comments.



Comment at: llvm/lib/Target/X86/X86TargetMachine.cpp:119-120
 
+  // Address spaces for 32 bit signed, 32 bit unsigned, and 64 bit pointers.
+  Ret += "-p253:32:32-p254:32:32-p255:64:64";
+

I may be wrong but this seems to not match what has been said in the responses 
to RFC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64931



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

r368326 - [clang][NFC] Move matcher ignoringElidableConstructorCall's tests to appropriate file.

[Yitzhak Mandelbaum via cfe-commits]

Author: ymandel
Date: Thu Aug  8 11:55:42 2019
New Revision: 368326

URL: http://llvm.org/viewvc/llvm-project?rev=368326=rev
Log:
[clang][NFC] Move matcher ignoringElidableConstructorCall's tests to 
appropriate file.

Summary:
`ignoringElidableConstructorCall` is a traversal matcher, but its tests are
grouped with narrowing-matcher tests. This revision moves them to the correct
file.

Reviewers: gribozavr

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp?rev=368326=368325=368326=diff
==
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp (original)
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp Thu Aug  8 
11:55:42 2019
@@ -603,91 +603,6 @@ TEST(Matcher, BindMatchedNodes) {

llvm::make_unique>("x")));
 }
 
-TEST(Matcher, IgnoresElidableConstructors) {
-  EXPECT_TRUE(
-  matches("struct H {};"
-  "template H B(T A);"
-  "void f() {"
-  "  H D1;"
-  "  D1 = B(B(1));"
-  "}",
-  cxxOperatorCallExpr(hasArgument(
-  1, callExpr(hasArgument(
- 0, ignoringElidableConstructorCall(callExpr()),
-  LanguageMode::Cxx11OrLater));
-  EXPECT_TRUE(
-  matches("struct H {};"
-  "template H B(T A);"
-  "void f() {"
-  "  H D1;"
-  "  D1 = B(1);"
-  "}",
-  cxxOperatorCallExpr(hasArgument(
-  1, callExpr(hasArgument(0, ignoringElidableConstructorCall(
- integerLiteral()),
-  LanguageMode::Cxx11OrLater));
-  EXPECT_TRUE(matches(
-  "struct H {};"
-  "H G();"
-  "void f() {"
-  "  H D = G();"
-  "}",
-  varDecl(hasInitializer(anyOf(
-  ignoringElidableConstructorCall(callExpr()),
-  
exprWithCleanups(has(ignoringElidableConstructorCall(callExpr())),
-  LanguageMode::Cxx11OrLater));
-}
-
-TEST(Matcher, IgnoresElidableInReturn) {
-  auto matcher = expr(ignoringElidableConstructorCall(declRefExpr()));
-  EXPECT_TRUE(matches("struct H {};"
-  "H f() {"
-  "  H g;"
-  "  return g;"
-  "}",
-  matcher, LanguageMode::Cxx11OrLater));
-  EXPECT_TRUE(notMatches("struct H {};"
- "H f() {"
- "  return H();"
- "}",
- matcher, LanguageMode::Cxx11OrLater));
-}
-
-TEST(Matcher, IgnoreElidableConstructorDoesNotMatchConstructors) {
-  EXPECT_TRUE(matches("struct H {};"
-  "void f() {"
-  "  H D;"
-  "}",
-  varDecl(hasInitializer(
-  
ignoringElidableConstructorCall(cxxConstructExpr(,
-  LanguageMode::Cxx11OrLater));
-}
-
-TEST(Matcher, IgnoresElidableDoesNotPreventMatches) {
-  EXPECT_TRUE(matches("void f() {"
-  "  int D = 10;"
-  "}",
-  expr(ignoringElidableConstructorCall(integerLiteral())),
-  LanguageMode::Cxx11OrLater));
-}
-
-TEST(Matcher, IgnoresElidableInVarInit) {
-  auto matcher =
-  varDecl(hasInitializer(ignoringElidableConstructorCall(callExpr(;
-  EXPECT_TRUE(matches("struct H {};"
-  "H G();"
-  "void f(H D = G()) {"
-  "  return;"
-  "}",
-  matcher, LanguageMode::Cxx11OrLater));
-  EXPECT_TRUE(matches("struct H {};"
-  "H G();"
-  "void f() {"
-  "  H D = G();"
-  "}",
-  matcher, LanguageMode::Cxx11OrLater));
-}
-
 TEST(Matcher, BindTheSameNameInAlternatives) {
   StatementMatcher matcher = anyOf(
 binaryOperator(hasOperatorName("+"),

Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp?rev=368326=368325=368326=diff
==
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp (original)
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp Thu Aug  8 
11:55:42 2019
@@ -1461,6 +1461,91 @@ TEST(HasImplicitDestinationType, DoesNot

[PATCH] D65963: [clang][NFC] Move matcher ignoringElidableConstructorCall's tests to appropriate file.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368326: [clang][NFC] Move matcher 
ignoringElidableConstructorCalls tests to… (authored by ymandel, 
committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65963?vs=214197=214203#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65963

Files:
  cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -603,91 +603,6 @@
llvm::make_unique>("x")));
 }
 
-TEST(Matcher, IgnoresElidableConstructors) {
-  EXPECT_TRUE(
-  matches("struct H {};"
-  "template H B(T A);"
-  "void f() {"
-  "  H D1;"
-  "  D1 = B(B(1));"
-  "}",
-  cxxOperatorCallExpr(hasArgument(
-  1, callExpr(hasArgument(
- 0, ignoringElidableConstructorCall(callExpr()),
-  LanguageMode::Cxx11OrLater));
-  EXPECT_TRUE(
-  matches("struct H {};"
-  "template H B(T A);"
-  "void f() {"
-  "  H D1;"
-  "  D1 = B(1);"
-  "}",
-  cxxOperatorCallExpr(hasArgument(
-  1, callExpr(hasArgument(0, ignoringElidableConstructorCall(
- integerLiteral()),
-  LanguageMode::Cxx11OrLater));
-  EXPECT_TRUE(matches(
-  "struct H {};"
-  "H G();"
-  "void f() {"
-  "  H D = G();"
-  "}",
-  varDecl(hasInitializer(anyOf(
-  ignoringElidableConstructorCall(callExpr()),
-  exprWithCleanups(has(ignoringElidableConstructorCall(callExpr())),
-  LanguageMode::Cxx11OrLater));
-}
-
-TEST(Matcher, IgnoresElidableInReturn) {
-  auto matcher = expr(ignoringElidableConstructorCall(declRefExpr()));
-  EXPECT_TRUE(matches("struct H {};"
-  "H f() {"
-  "  H g;"
-  "  return g;"
-  "}",
-  matcher, LanguageMode::Cxx11OrLater));
-  EXPECT_TRUE(notMatches("struct H {};"
- "H f() {"
- "  return H();"
- "}",
- matcher, LanguageMode::Cxx11OrLater));
-}
-
-TEST(Matcher, IgnoreElidableConstructorDoesNotMatchConstructors) {
-  EXPECT_TRUE(matches("struct H {};"
-  "void f() {"
-  "  H D;"
-  "}",
-  varDecl(hasInitializer(
-  ignoringElidableConstructorCall(cxxConstructExpr(,
-  LanguageMode::Cxx11OrLater));
-}
-
-TEST(Matcher, IgnoresElidableDoesNotPreventMatches) {
-  EXPECT_TRUE(matches("void f() {"
-  "  int D = 10;"
-  "}",
-  expr(ignoringElidableConstructorCall(integerLiteral())),
-  LanguageMode::Cxx11OrLater));
-}
-
-TEST(Matcher, IgnoresElidableInVarInit) {
-  auto matcher =
-  varDecl(hasInitializer(ignoringElidableConstructorCall(callExpr(;
-  EXPECT_TRUE(matches("struct H {};"
-  "H G();"
-  "void f(H D = G()) {"
-  "  return;"
-  "}",
-  matcher, LanguageMode::Cxx11OrLater));
-  EXPECT_TRUE(matches("struct H {};"
-  "H G();"
-  "void f() {"
-  "  H D = G();"
-  "}",
-  matcher, LanguageMode::Cxx11OrLater));
-}
-
 TEST(Matcher, BindTheSameNameInAlternatives) {
   StatementMatcher matcher = anyOf(
 binaryOperator(hasOperatorName("+"),
Index: cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -1461,6 +1461,91 @@
unless(anything());
 }
 
+TEST(Matcher, IgnoresElidableConstructors) {
+  EXPECT_TRUE(
+  matches("struct H {};"
+  "template H B(T A);"
+  "void f() {"
+  "  H D1;"
+  "  D1 = B(B(1));"
+  "}",
+  cxxOperatorCallExpr(hasArgument(
+  1, callExpr(hasArgument(
+ 0, ignoringElidableConstructorCall(callExpr()),
+  LanguageMode::Cxx11OrLater));
+  EXPECT_TRUE(
+

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

[Amy Huang via Phabricator via cfe-commits]

akhuang updated this revision to Diff 214202.
akhuang added a comment.

Remove test case changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64931

Files:
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/X86.h
  clang/test/CodeGen/iamcu-abi.c
  clang/test/CodeGen/target-data.c
  llvm/lib/Target/X86/X86TargetMachine.cpp

Index: llvm/lib/Target/X86/X86TargetMachine.cpp
===
--- llvm/lib/Target/X86/X86TargetMachine.cpp
+++ llvm/lib/Target/X86/X86TargetMachine.cpp
@@ -116,6 +116,9 @@
   !TT.isArch64Bit())
 Ret += "-p:32:32";
 
+  // Address spaces for 32 bit signed, 32 bit unsigned, and 64 bit pointers.
+  Ret += "-p253:32:32-p254:32:32-p255:64:64";
+
   // Some ABIs align 64 bit integers and doubles to 64 bits, others to 32.
   if (TT.isArch64Bit() || TT.isOSWindows() || TT.isOSNaCl())
 Ret += "-i64:64";
Index: clang/test/CodeGen/target-data.c
===
--- clang/test/CodeGen/target-data.c
+++ clang/test/CodeGen/target-data.c
@@ -1,22 +1,22 @@
 // RUN: %clang_cc1 -triple i686-unknown-unknown -emit-llvm -o - %s | \
 // RUN: FileCheck --check-prefix=I686-UNKNOWN %s
-// I686-UNKNOWN: target datalayout = "e-m:e-p:32:32-f64:32:64-f80:32-n8:16:32-S128"
+// I686-UNKNOWN: target datalayout = "e-m:e-p:32:32-p253:32:32-p254:32:32-p255:64:64-f64:32:64-f80:32-n8:16:32-S128"
 
 // RUN: %clang_cc1 -triple i686-apple-darwin9 -emit-llvm -o - %s | \
 // RUN: FileCheck --check-prefix=I686-DARWIN %s
-// I686-DARWIN: target datalayout = "e-m:o-p:32:32-f64:32:64-f80:128-n8:16:32-S128"
+// I686-DARWIN: target datalayout = "e-m:o-p:32:32-p253:32:32-p254:32:32-p255:64:64-f64:32:64-f80:128-n8:16:32-S128"
 
 // RUN: %clang_cc1 -triple i686-unknown-win32 -emit-llvm -o - %s | \
 // RUN: FileCheck --check-prefix=I686-WIN32 %s
-// I686-WIN32: target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
+// I686-WIN32: target datalayout = "e-m:x-p:32:32-p253:32:32-p254:32:32-p255:64:64-i64:64-f80:32-n8:16:32-a:0:32-S32"
 
 // RUN: %clang_cc1 -triple i686-unknown-cygwin -emit-llvm -o - %s | \
 // RUN: FileCheck --check-prefix=I686-CYGWIN %s
-// I686-CYGWIN: target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
+// I686-CYGWIN: target datalayout = "e-m:x-p:32:32-p253:32:32-p254:32:32-p255:64:64-i64:64-f80:32-n8:16:32-a:0:32-S32"
 
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s | \
 // RUN: FileCheck --check-prefix=X86_64 %s
-// X86_64: target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+// X86_64: target datalayout = "e-m:e-p253:32:32-p254:32:32-p255:64:64-i64:64-f80:128-n8:16:32:64-S128"
 
 // RUN: %clang_cc1 -triple xcore-unknown-unknown -emit-llvm -o - %s | \
 // RUN: FileCheck --check-prefix=XCORE %s
@@ -88,11 +88,11 @@
 
 // RUN: %clang_cc1 -triple i686-nacl -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=I686-NACL
-// I686-NACL: target datalayout = "e-m:e-p:32:32-i64:64-n8:16:32-S128"
+// I686-NACL: target datalayout = "e-m:e-p:32:32-p253:32:32-p254:32:32-p255:64:64-i64:64-n8:16:32-S128"
 
 // RUN: %clang_cc1 -triple x86_64-nacl -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=X86_64-NACL
-// X86_64-NACL: target datalayout = "e-m:e-p:32:32-i64:64-n8:16:32:64-S128"
+// X86_64-NACL: target datalayout = "e-m:e-p:32:32-p253:32:32-p254:32:32-p255:64:64-i64:64-n8:16:32:64-S128"
 
 // RUN: %clang_cc1 -triple arm-nacl -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=ARM-NACL
Index: clang/test/CodeGen/iamcu-abi.c
===
--- clang/test/CodeGen/iamcu-abi.c
+++ clang/test/CodeGen/iamcu-abi.c
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -triple i386-pc-elfiamcu -emit-llvm -o - %s | FileCheck %s
 
-// CHECK: target datalayout = "e-m:e-p:32:32-i64:32-f64:32-f128:32-n8:16:32-a:0:32-S32"
+// CHECK: target datalayout = "e-m:e-p:32:32-p253:32:32-p254:32:32-p255:64:64-i64:32-f64:32-f128:32-n8:16:32-a:0:32-S32"
 // CHECK: target triple = "i386-pc-elfiamcu"
 
 
Index: clang/lib/Basic/Targets/X86.h
===
--- clang/lib/Basic/Targets/X86.h
+++ clang/lib/Basic/Targets/X86.h
@@ -340,7 +340,7 @@
 LongDoubleWidth = 96;
 LongDoubleAlign = 32;
 SuitableAlign = 128;
-resetDataLayout("e-m:e-p:32:32-f64:32:64-f80:32-n8:16:32-S128");
+resetDataLayout("e-m:e-p:32:32-p253:32:32-p254:32:32-p255:64:64-f64:32:64-f80:32-n8:16:32-S128");
 SizeType = UnsignedInt;
 PtrDiffType = SignedInt;
 IntPtrType = SignedInt;
@@ -440,7 +440,7 @@
   UseSignedCharForObjCBool = false;
 SizeType = UnsignedLong;
 IntPtrType = SignedLong;
-resetDataLayout("e-m:o-p:32:32-f64:32:64-f80:128-n8:16:32-S128");
+

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 214186.
Charusso added a comment.

- The call as `getAsCXXRecordDecl()` sometimes run into an assertion failure, 
so we need to avoid it:

  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:333:
  ExprEngine::createTemporaryRegionIfNeeded():
  Assertion `!InitValWithAdjustments.getAs() || 
Loc::isLocType(Result->getType()) || Result->getType()->isMemberPointerType()' 
failed.


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

https://reviews.llvm.org/D65889

Files:
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/test/Analysis/cast-value.cpp

Index: clang/test/Analysis/cast-value.cpp
===
--- clang/test/Analysis/cast-value.cpp
+++ clang/test/Analysis/cast-value.cpp
@@ -20,14 +20,26 @@
 const X *cast_or_null(Y Value);
 
 template 
-const X *dyn_cast_or_null(Y Value);
+const X *dyn_cast_or_null(Y *Value);
+
+template 
+const X *dyn_cast_or_null(const Y );
 } // namespace llvm
 
-using namespace llvm;
+namespace clang {
+struct Shape {
+  template 
+  const T *castAs() const;
 
-class Shape {};
+  template 
+  const T *getAs() const;
+};
 class Triangle : public Shape {};
 class Circle : public Shape {};
+} // namespace clang
+
+using namespace llvm;
+using namespace clang;
 
 namespace test_cast {
 void evalLogic(const Shape *S) {
@@ -91,8 +103,44 @@
   if (!S)
 clang_analyzer_eval(!C); // logic-warning {{TRUE}}
 }
+} // namespace test_dyn_cast_or_null
 
-void evalNonNullParamNonNullReturn(const Shape *S) {
+namespace test_cast_as {
+void evalLogic(const Shape *S) {
+  const Circle *C = S->castAs();
+  clang_analyzer_numTimesReached(); // logic-warning {{1}}
+
+  if (S && C)
+clang_analyzer_eval(C == S);
+  // logic-warning@-1 {{TRUE}}
+
+  if (S && !C)
+clang_analyzer_warnIfReached(); // no-warning
+
+  if (!S)
+clang_analyzer_warnIfReached(); // no-warning
+}
+} // namespace test_cast_as
+
+namespace test_get_as {
+void evalLogic(const Shape *S) {
+  const Circle *C = S->getAs();
+  clang_analyzer_numTimesReached(); // logic-warning {{2}}
+
+  if (S && C)
+clang_analyzer_eval(C == S);
+  // logic-warning@-1 {{TRUE}}
+
+  if (S && !C)
+clang_analyzer_warnIfReached(); // logic-warning {{REACHABLE}}
+
+  if (!S)
+clang_analyzer_warnIfReached(); // no-warning
+}
+} // namespace test_get_as
+
+namespace test_notes {
+void evalNonNullParamNonNullReturnReference(const Shape ) {
   const auto *C = dyn_cast_or_null(S);
   // expected-note@-1 {{Assuming dynamic cast from 'Shape' to 'Circle' succeeds}}
   // expected-note@-2 {{Assuming pointer value is null}}
@@ -105,6 +153,19 @@
   // logic-warning@-4 {{Division by zero}}
 }
 
+void evalNonNullParamNonNullReturn(const Shape *S) {
+  const auto *C = cast(S);
+  // expected-note@-1 {{Dynamic cast from 'Shape' to 'Circle' succeeds}}
+  // expected-note@-2 {{Assuming pointer value is null}}
+  // expected-note@-3 {{'C' initialized here}}
+
+  (void)(1 / !(bool)C);
+  // expected-note@-1 {{'C' is non-null}}
+  // expected-note@-2 {{Division by zero}}
+  // expected-warning@-3 {{Division by zero}}
+  // logic-warning@-4 {{Division by zero}}
+}
+
 void evalNonNullParamNullReturn(const Shape *S) {
   const auto *C = dyn_cast_or_null(S);
   // expected-note@-1 {{Assuming dynamic cast from 'Shape' to 'Circle' fails}}
@@ -134,4 +195,40 @@
   // expected-warning@-2 {{Division by zero}}
   // logic-warning@-3 {{Division by zero}}
 }
-} // namespace test_dyn_cast_or_null
+
+void evalZeroParamNonNullReturnPointer(const Shape *S) {
+  const auto *C = S->castAs();
+  // expected-note@-1 {{Assuming pointer value is null}}
+  // expected-note@-2 {{Dynamic cast to 'Circle' succeeds}}
+  // expected-note@-3 {{'C' initialized here}}
+
+  (void)(1 / !(bool)C);
+  // expected-note@-1 {{'C' is non-null}}
+  // expected-note@-2 {{Division by zero}}
+  // expected-warning@-3 {{Division by zero}}
+  // logic-warning@-4 {{Division by zero}}
+}
+
+void evalZeroParamNonNullReturn(const Shape ) {
+  const auto *C = S.castAs();
+  // expected-note@-1 {{Dynamic cast to 'Circle' succeeds}}
+  // expected-note@-2 {{'C' initialized here}}
+
+  (void)(1 / !(bool)C);
+  // expected-note@-1 {{'C' is non-null}}
+  // expected-note@-2 {{Division by zero}}
+  // expected-warning@-3 {{Division by zero}}
+  // logic-warning@-4 {{Division by zero}}
+}
+
+void evalZeroParamNullReturn(const Shape ) {
+  const auto *C = S.getAs();
+  // expected-note@-1 {{Assuming dynamic cast to 'Circle' fails}}
+  // expected-note@-2 {{'C' initialized to a null pointer value}}
+
+  (void)(1 / (bool)C);
+  // expected-note@-1 {{Division by zero}}
+  // expected-warning@-2 {{Division by zero}}
+  // logic-warning@-3 {{Division by zero}}
+}
+} // namespace test_notes
Index: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
+++ 

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

[Amy Huang via Phabricator via cfe-commits]

akhuang added a comment.

For some reason the tests were failing before without the datalayout change? 
I'm not sure why, but I changed them back and they're fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64931



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

[PATCH] D65963: [clang][NFC] Move matcher ignoringElidableConstructorCall's tests to appropriate file.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr accepted this revision.
gribozavr 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/D65963/new/

https://reviews.llvm.org/D65963



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

[PATCH] D65919: [clang-tidy] Add llvm-prefer-register-over-unsigned check and apply it to LLVM

[Daniel Sanders via Phabricator via cfe-commits]

dsanders updated this revision to Diff 214194.
dsanders marked 2 inline comments as done.
dsanders added a comment.

UsingDirectiveDecl -> auto
` -> ``
Split LLVM changes into another patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65919

Files:
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp
  clang-tools-extra/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-register-over-unsigned.rst
  clang-tools-extra/test/clang-tidy/llvm-prefer-register-over-unsigned.cpp

Index: clang-tools-extra/test/clang-tidy/llvm-prefer-register-over-unsigned.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-prefer-register-over-unsigned.cpp
@@ -0,0 +1,57 @@
+// RUN: %check_clang_tidy %s llvm-prefer-register-over-unsigned %t
+
+namespace llvm {
+class Register {
+public:
+  operator unsigned();
+};
+} // end namespace llvm
+
+llvm::Register getReg();
+
+void apply_1() {
+  unsigned Reg = getReg();
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: var 'Reg' is 'unsigned int' but holds a register [llvm-prefer-register-over-unsigned]
+  // CHECK-FIXES: apply_1()
+  // CHECK-FIXES-NEXT: llvm::Register Reg = getReg();
+}
+
+void apply_2() {
+  using namespace llvm;
+  unsigned Reg = getReg();
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: var 'Reg' is 'unsigned int' but holds a register [llvm-prefer-register-over-unsigned]
+  // CHECK-FIXES: apply_2()
+  // CHECK-FIXES-NEXT: using namespace llvm;
+  // CHECK-FIXES-NEXT: Register Reg = getReg();
+}
+
+namespace llvm {
+void apply_3() {
+  unsigned Reg = getReg();
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: var 'Reg' is 'unsigned int' but holds a register [llvm-prefer-register-over-unsigned]
+  // CHECK-FIXES: apply_3()
+  // CHECK-FIXES-NEXT: Register Reg = getReg();
+}
+} // end namespace llvm
+
+void done_1() {
+  llvm::Register Reg = getReg();
+  // CHECK-FIXES: done_1()
+  // CHECK-FIXES-NEXT: llvm::Register Reg = getReg();
+}
+
+void done_2() {
+  using namespace llvm;
+  Register Reg = getReg();
+  // CHECK-FIXES: done_2()
+  // CHECK-FIXES-NEXT: using namespace llvm;
+  // CHECK-FIXES-NEXT: Register Reg = getReg();
+}
+
+namespace llvm {
+void done_3() {
+  Register Reg = getReg();
+  // CHECK-FIXES: done_3()
+  // CHECK-FIXES-NEXT: Register Reg = getReg();
+}
+} // end namespace llvm
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-register-over-unsigned.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-register-over-unsigned.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - llvm-prefer-register-over-unsigned
+
+llvm-prefer-register-over-unsigned
+==
+
+Finds historical use of ``unsigned`` to hold vregs and physregs and rewrites
+them to use ``Register``.
+
+Currently this works by finding all variables of unsigned integer type whose
+initialize begins with an implicit cast from ``Register`` to ``unsigned``.
+
+.. code-block:: c++
+
+  void example(MachineOperand ) {
+unsigned Reg = MO.getReg();
+...
+  }
+
+becomes:
+
+.. code-block:: c++
+
+  void example(MachineOperand ) {
+Register Reg = MO.getReg();
+...
+  }
+
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -274,6 +274,7 @@
llvm-include-order
llvm-namespace-comment
llvm-prefer-isa-or-dyn-cast-in-conditionals
+   llvm-prefer-register-over-unsigned
llvm-twine-local
misc-definitions-in-headers
misc-misplaced-const
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -79,6 +79,12 @@
   Finds uses of deprecated Googletest APIs with names containing ``case`` and
   replaces them with equivalent APIs with ``suite``.
 
+- New :doc:`llvm-prefer-register-over-unsigned
+  ` check.
+
+  Finds historical use of ``unsigned`` to hold vregs and physregs and rewrites
+  them to use ``Register``
+
 
 Improvements to include-fixer
 -
Index: clang-tools-extra/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.h
@@ -0,0 +1,36 @@
+//===--- PreferRegisterOverUnsignedCheck.h - clang-tidy -*- C++ -*-===//
+//
+// 

[PATCH] D65919: [clang-tidy] Add llvm-prefer-register-over-unsigned check and apply it to LLVM

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

I think patch should be split at least on Clang-tidy check and results of its 
run on LLVM code. Probably per-target patches is even better solution.




Comment at: 
clang-tools-extra/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp:48
+Replacement = "Register";
+for (const UsingDirectiveDecl *UsingDirective: Context->using_directives())
+  if (UsingDirective->getNominatedNamespace()

You could use const auto * for iterators.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:85
+
+  Finds historical use of `unsigned` to hold vregs and physregs and rewrites
+  them to use `Register`

Please use double back-ticks to highlight language constructs. Same in 
documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65919



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

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: clang/lib/Basic/Targets/OSTargets.h:771-772
 } else if (Triple.getArch() == llvm::Triple::x86) {
-  this->resetDataLayout("e-m:e-p:32:32-i64:64-n8:16:32-S128");
+  this->resetDataLayout("e-m:e-p:32:32-p253:32:32-p254:32:32-p255:64:64-"
+"i64:64-n8:16:32-S128");
 } else if (Triple.getArch() == llvm::Triple::x86_64) {

lebedev.ri wrote:
> I'd expect that this should be guarded by whatever flag is used for ms 
> extensions.
> Put differently, i i'm not sure that when those extensions are not enabled, 
> the datalayout should be changed?
As discussed in the RFC, other people find these address spaces generally 
useful, and there is no need to limit them to just Windows or *-windows-msvc 
targets. Making it conditional would require mirroring the same conditional 
into LLVM, because LLVM and clang have to agree on data layout.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64931



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

[PATCH] D65906: [clang-scan-deps] Fix edge cases in the minimizer

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

In D65906#1620034 , @arphaman wrote:

> Regarding the invisible characters before `#ifdef`, are you sure that's the 
> right behavior?


Should we then copy these invisible characters to the minimized output? Believe 
it or not, these characters are there in our codebase since 2013, and never 
clang has complained about it :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D65906



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

[PATCH] D65963: [clang][NFC] Move matcher ignoringElidableConstructorCall's tests to appropriate file.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added a reviewer: gribozavr.
Herald added a project: clang.

`ignoringElidableConstructorCall` is a traversal matcher, but its tests are
grouped with narrowing-matcher tests. This revision moves them to the correct
file.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65963

Files:
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -1461,6 +1461,91 @@
unless(anything());
 }
 
+TEST(Matcher, IgnoresElidableConstructors) {
+  EXPECT_TRUE(
+  matches("struct H {};"
+  "template H B(T A);"
+  "void f() {"
+  "  H D1;"
+  "  D1 = B(B(1));"
+  "}",
+  cxxOperatorCallExpr(hasArgument(
+  1, callExpr(hasArgument(
+ 0, ignoringElidableConstructorCall(callExpr()),
+  LanguageMode::Cxx11OrLater));
+  EXPECT_TRUE(
+  matches("struct H {};"
+  "template H B(T A);"
+  "void f() {"
+  "  H D1;"
+  "  D1 = B(1);"
+  "}",
+  cxxOperatorCallExpr(hasArgument(
+  1, callExpr(hasArgument(0, ignoringElidableConstructorCall(
+ integerLiteral()),
+  LanguageMode::Cxx11OrLater));
+  EXPECT_TRUE(matches(
+  "struct H {};"
+  "H G();"
+  "void f() {"
+  "  H D = G();"
+  "}",
+  varDecl(hasInitializer(anyOf(
+  ignoringElidableConstructorCall(callExpr()),
+  exprWithCleanups(has(ignoringElidableConstructorCall(callExpr())),
+  LanguageMode::Cxx11OrLater));
+}
+
+TEST(Matcher, IgnoresElidableInReturn) {
+  auto matcher = expr(ignoringElidableConstructorCall(declRefExpr()));
+  EXPECT_TRUE(matches("struct H {};"
+  "H f() {"
+  "  H g;"
+  "  return g;"
+  "}",
+  matcher, LanguageMode::Cxx11OrLater));
+  EXPECT_TRUE(notMatches("struct H {};"
+ "H f() {"
+ "  return H();"
+ "}",
+ matcher, LanguageMode::Cxx11OrLater));
+}
+
+TEST(Matcher, IgnoreElidableConstructorDoesNotMatchConstructors) {
+  EXPECT_TRUE(matches("struct H {};"
+  "void f() {"
+  "  H D;"
+  "}",
+  varDecl(hasInitializer(
+  ignoringElidableConstructorCall(cxxConstructExpr(,
+  LanguageMode::Cxx11OrLater));
+}
+
+TEST(Matcher, IgnoresElidableDoesNotPreventMatches) {
+  EXPECT_TRUE(matches("void f() {"
+  "  int D = 10;"
+  "}",
+  expr(ignoringElidableConstructorCall(integerLiteral())),
+  LanguageMode::Cxx11OrLater));
+}
+
+TEST(Matcher, IgnoresElidableInVarInit) {
+  auto matcher =
+  varDecl(hasInitializer(ignoringElidableConstructorCall(callExpr(;
+  EXPECT_TRUE(matches("struct H {};"
+  "H G();"
+  "void f(H D = G()) {"
+  "  return;"
+  "}",
+  matcher, LanguageMode::Cxx11OrLater));
+  EXPECT_TRUE(matches("struct H {};"
+  "H G();"
+  "void f() {"
+  "  H D = G();"
+  "}",
+  matcher, LanguageMode::Cxx11OrLater));
+}
+
 TEST(IgnoringImplicit, MatchesImplicit) {
   EXPECT_TRUE(matches("class C {}; C a = C();",
   varDecl(has(ignoringImplicit(cxxConstructExpr());
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -603,91 +603,6 @@
llvm::make_unique>("x")));
 }
 
-TEST(Matcher, IgnoresElidableConstructors) {
-  EXPECT_TRUE(
-  matches("struct H {};"
-  "template H B(T A);"
-  "void f() {"
-  "  H D1;"
-  "  D1 = B(B(1));"
-  "}",
-  cxxOperatorCallExpr(hasArgument(
-  1, callExpr(hasArgument(
- 0, ignoringElidableConstructorCall(callExpr()),
-  LanguageMode::Cxx11OrLater));
-  EXPECT_TRUE(
-  matches("struct H {};"
-  "template H B(T A);"
-  "void f() {"
-  "  H D1;"
-  "  D1 =