Re: Upgrade and fix clang-format-vs

[Zachary Turner via cfe-commits]

Does running vcvarsall put nuget on the path?

What if we require the user to specify the path to nuget in some CMake
variable? -DMSVC_NUGET_PATH=foo?
On Fri, Nov 25, 2016 at 6:58 PM Antonio Maiorano 
wrote:

> Ah, no, that's not what I meant. The required referenced assemblies are
> versions that are normally installed with VS 2010.
>
> The first time I worked on this, I had upgraded the referenced assemblies
> to the ones that ship with VS 2015, but then there was question of whether
> or not the VSIX would continue to work with VS 2010/2012/2013. I'm not sure
> if it would work, but I guess I can try to figure that out.
>
> In any case, what I discovered is that the "right" way to do things to
> make sure your extension compiles in future versions of VS is to use NuGet
> to automatically pull in the required assemblies, or to check them in and
> reference them directly. The former would be better except for the problem
> of CLI builds as I described in my earlier email.
>
>
>
> On Fri, 25 Nov 2016 at 21:47 Zachary Turner  wrote:
>
> Sorry, i think I misunderstood the original option 1. I interpreted it as
> just committing changes to the vsix manifest to reference a specific
> version of the assembly which we assume to be present since it should be
> automatically installed with vs 2015. Is this not possible? Can't we just
> point the manifest to the version installed with vs?
> On Fri, Nov 25, 2016 at 6:20 PM Antonio Maiorano 
> wrote:
>
> Hi again,
>
> I've made the changes so that the required assemblies are committed, so
> now we can build the clang-format-vsix with just VS 2015. Since the patch
> set is around 9 mb, I'm providing a link to it on my Dropbox (if you'd
> rather I attach it, let me know):
>
>
> https://dl.dropboxusercontent.com/u/10504225/llvm-patches/0001-Fix-VS2015-build-of-clang-format-vsix-by-committing-.patch
>
> Note that it's a git patch set, for which I followed the instructions here
> .
>
> Thanks.
>
> On Thu, 24 Nov 2016 at 15:45 Antonio Maiorano  wrote:
>
> Okay, that's fine, I'll go for that and if all looks good, will attach a
> patch.
>
> Thanks.
>
> On Thu, 24 Nov 2016 at 15:09 Zachary Turner  wrote:
>
> I would use the first solution. We lock ourselves to specific versions of
> vs, so i think it's fine to do the same with the assemblies and deal with
> it only when we upgrade
> On Thu, Nov 24, 2016 at 11:29 AM Antonio Maiorano 
> wrote:
>
> Hi Hans,
>
> I saw that on September 15th, you checked in a change: clang-format VS
> plugin: upgrade the project files to VS2015.
>
> When I open the latest version of ClangFormat.sln on a machine that has
> only VS 2015, it doesn't build. The reason is that some of the referenced
> assemblies are from VS 2010.
>
>   Include="Microsoft.VisualStudio.Editor, Version=10.0.0.0, Culture=neutral,
> PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL" />
>
> What happens is that when building, these specific assemblies are not
> found. I suspect you have VS 2010 installed on your machine, which is why
> you don't get these build errors.
>
> The recommended way to handle this
> 
> is to make use of NuGet to have it automatically download the required
> assemblies. I have made the changes locally to get this working, and it
> works great when building ClangFormat.sln from within Visual Studio;
> however, building from the CLI doesn't work out of the box because you must
> explicitly run 'nuget.exe restore ClangFormat.sln' before running msbuild
> (or devenv.exe in our case). The problem is that nuget.exe isn't something
> that's easily found/accessible on Windows, even once installed as an
> extension in VS. This is a known problem
>  and the current best solution
> requires downloading and making nuget.exe available in PATH
> .
>
> So now i'm faced with figuring out how best to solve this so that the
> custom build step in this CMakeLists.txt
> 
>  that
> runs devenv doesn't fail:
>
> devenv "${CMAKE_CURRENT_SOURCE_DIR}/ClangFormat.sln" /Build Release
>
> There are a few options:
>
> 1) Forget NuGet and just commit the referenced assemblies. This is the
> simplest solution, but is more annoying to manage if we want to upgrade the
> versions of these assemblies later.
>
> 2) Commit nuget.exe to the repo and just use it. This is simple enough,
> but I'm not sure how people feel about committing binaries, and it would be
> frozen at whatever version we commit.
>
> 3) Do some form of wget to grab the latest nuget.exe from "
> https://nuget.org/nuget.exe; in CMake and invoke it. I'm not yet sure
> 

Re: Upgrade and fix clang-format-vs

[Zachary Turner via cfe-commits]

Sorry, i think I misunderstood the original option 1. I interpreted it as
just committing changes to the vsix manifest to reference a specific
version of the assembly which we assume to be present since it should be
automatically installed with vs 2015. Is this not possible? Can't we just
point the manifest to the version installed with vs?
On Fri, Nov 25, 2016 at 6:20 PM Antonio Maiorano 
wrote:

> Hi again,
>
> I've made the changes so that the required assemblies are committed, so
> now we can build the clang-format-vsix with just VS 2015. Since the patch
> set is around 9 mb, I'm providing a link to it on my Dropbox (if you'd
> rather I attach it, let me know):
>
>
> https://dl.dropboxusercontent.com/u/10504225/llvm-patches/0001-Fix-VS2015-build-of-clang-format-vsix-by-committing-.patch
>
> Note that it's a git patch set, for which I followed the instructions here
> .
>
> Thanks.
>
> On Thu, 24 Nov 2016 at 15:45 Antonio Maiorano  wrote:
>
> Okay, that's fine, I'll go for that and if all looks good, will attach a
> patch.
>
> Thanks.
>
> On Thu, 24 Nov 2016 at 15:09 Zachary Turner  wrote:
>
> I would use the first solution. We lock ourselves to specific versions of
> vs, so i think it's fine to do the same with the assemblies and deal with
> it only when we upgrade
> On Thu, Nov 24, 2016 at 11:29 AM Antonio Maiorano 
> wrote:
>
> Hi Hans,
>
> I saw that on September 15th, you checked in a change: clang-format VS
> plugin: upgrade the project files to VS2015.
>
> When I open the latest version of ClangFormat.sln on a machine that has
> only VS 2015, it doesn't build. The reason is that some of the referenced
> assemblies are from VS 2010.
>
>   Include="Microsoft.VisualStudio.Editor, Version=10.0.0.0, Culture=neutral,
> PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL" />
>
> What happens is that when building, these specific assemblies are not
> found. I suspect you have VS 2010 installed on your machine, which is why
> you don't get these build errors.
>
> The recommended way to handle this
> 
> is to make use of NuGet to have it automatically download the required
> assemblies. I have made the changes locally to get this working, and it
> works great when building ClangFormat.sln from within Visual Studio;
> however, building from the CLI doesn't work out of the box because you must
> explicitly run 'nuget.exe restore ClangFormat.sln' before running msbuild
> (or devenv.exe in our case). The problem is that nuget.exe isn't something
> that's easily found/accessible on Windows, even once installed as an
> extension in VS. This is a known problem
>  and the current best solution
> requires downloading and making nuget.exe available in PATH
> .
>
> So now i'm faced with figuring out how best to solve this so that the
> custom build step in this CMakeLists.txt
> 
>  that
> runs devenv doesn't fail:
>
> devenv "${CMAKE_CURRENT_SOURCE_DIR}/ClangFormat.sln" /Build Release
>
> There are a few options:
>
> 1) Forget NuGet and just commit the referenced assemblies. This is the
> simplest solution, but is more annoying to manage if we want to upgrade the
> versions of these assemblies later.
>
> 2) Commit nuget.exe to the repo and just use it. This is simple enough,
> but I'm not sure how people feel about committing binaries, and it would be
> frozen at whatever version we commit.
>
> 3) Do some form of wget to grab the latest nuget.exe from "
> https://nuget.org/nuget.exe; in CMake and invoke it. I'm not yet sure
> what's the simplest way to do this. Powershell could be used, but there are
> security annoyances to deal with.
>
> That's all I can come up with so far. Would love to hear from you guys if
> you have any ideas or opinions on this. If you want I can send you a patch
> of what I've got so far if that helps.
>
> Thanks,
>
> Antonio Maiorano
>
>
>
> On Thu, 15 Sep 2016 at 19:35 Antonio Maiorano  wrote:
>
> Sorry I haven't had a chance to get back to this. Things got busy at work.
> I do plan to get back to this as I'm hoping to add some features to this
> extension :)
> On Thu, Sep 15, 2016 at 7:31 PM Zachary Turner  wrote:
>
> Strange.  FWIW you can dump all the variables that are present in your
> environment.  You need to go to Tools -> Options -> Projects and Solutions
> -> Build and Run and choose either Normal, Detailed, or Diagnostic for the
> MSBuild project build output verbosity.  Then in the output window you will
> get a ton of spam, some of which is the set of all MSBuild variables you
> can take advantage of.
>
> On Thu, Sep 15, 2016 at 

[PATCH] D27116: Fix crash when using __has_nothrow_copy with inherited constructors

[Richard Smith via Phabricator via cfe-commits]

rsmith added a comment.

Do we still need this after https://reviews.llvm.org/D23765 lands?


https://reviews.llvm.org/D27116



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

[PATCH] D27140: Allow clang to write compilation database records

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg created this revision.
joerg added reviewers: klimek, rsmith.
joerg added a subscriber: cfe-commits.
joerg set the repository for this revision to rL LLVM.
joerg added a dependency: D27138: Extend CompilationDatabase by a field for the 
output filename.

When integrating compilation database output into existing build systems, two 
approaches dominate so far. Ad-hoc implementation of the JSON output rules or 
using compiler wrappers. This patch adds a new option "-MJ foo.json" which 
gives a slightly cleaned up compilation record. The output is a fragment, i.e. 
you still need to add the array markers, but it allows multiple files to be 
easy merged. This way the only change in a build system is adding the option 
with potentially a per-target output file and merging the files with something 
like `(echo '['; cat *.o.json; echo ']' > compilation_database.json`.

The current implementation has two issues:

1. It doesn't honor -###. This would be easily fixable though.
2. It opens the output file more than once. That's why it is currently using 
append mode. Fixing this requires either moving it to a different part in the 
processing chain or storing the stream in an appropiate place.

The output record currently depends on https://reviews.llvm.org/D27138, but 
would be easily adjustable if necessary. I'm aware of the missing test cases, 
will provide them once the architectural side is clear.


Repository:
  rL LLVM

https://reviews.llvm.org/D27140

Files:
  include/clang/Driver/Options.td
  lib/Driver/Tools.cpp

Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -266,6 +266,8 @@
 MetaVarName<"">;
 def MG : Flag<["-"], "MG">, Group, Flags<[CC1Option]>,
 HelpText<"Add missing headers to depfile">;
+def MJ : JoinedOrSeparate<["-"], "MJ">, Group,
+HelpText<"Write a compilation database entry per input">;
 def MP : Flag<["-"], "MP">, Group, Flags<[CC1Option]>,
 HelpText<"Create phony target for each dependency (other than main file)">;
 def MQ : JoinedOrSeparate<["-"], "MQ">, Group, Flags<[CC1Option]>,
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4002,6 +4002,72 @@
 CmdArgs.push_back("-KPIC");
 }
 
+static void QuoteJSONString(llvm::raw_fd_ostream , StringRef Str) {
+  Stream << "\"";
+  if (Str.find_first_of("\\\"") == Str.npos) {
+Stream << Str;
+  } else {
+for (auto ch: Str) {
+  if (ch == '\\' || ch == '"')
+Stream << '\\';
+  Stream << ch;
+}
+  }
+  Stream << "\"";
+}
+
+static void DumpCompilationDatabase(const Driver , StringRef Filename, StringRef Target, const InputInfo ,
+const InputInfo , const ArgList ) {
+  std::error_code EC;
+  llvm::raw_fd_ostream File(Filename, EC, llvm::sys::fs::F_Text | llvm::sys::fs::F_Append);
+  if (EC) {
+//errs() << "Failed to open " << Filename << ": " << EC.message() << "\n" ;
+return;
+  }
+  SmallString<128> Buf;
+  if (llvm::sys::fs::current_path(Buf))
+Buf = ".";
+  File << "{ \"directory\": ";
+  QuoteJSONString(File, Buf);
+  File << ", \"file\": ";
+  QuoteJSONString(File, Input.getFilename());
+  File << ", \"output\": ";
+  QuoteJSONString(File, Output.getFilename());
+
+  File << ", \"arguments\": [";
+  QuoteJSONString(File, D.ClangExecutable);
+  File << ", ";
+  Buf = "-x";
+  Buf += types::getTypeName(Input.getType());
+  QuoteJSONString(File, Buf);
+  File << ", ";
+  QuoteJSONString(File, Input.getFilename());
+  for (auto : Args) {
+auto  = A->getOption();
+// Skip language selection, which is positional.
+if (O.getID() == options::OPT_x)
+  continue;
+// Skip writing dependency output and the compiliation database itself.
+if (O.getGroup().isValid() && O.getGroup().getID() == options::OPT_M_Group)
+  continue;
+// Skip inputs.
+if (O.getKind() == Option::InputClass)
+  continue;
+// All other arguments are quoted and appended.
+ArgStringList ASL;
+A->render(Args, ASL);
+for (auto : ASL) {
+  File << ", ";
+  QuoteJSONString(File, it);
+}
+  }
+  File << ", ";
+  Buf = "--target=";
+  Buf += Target;
+  QuoteJSONString(File, Buf);
+  File << "]},\n";
+}
+
 void Clang::ConstructJob(Compilation , const JobAction ,
  const InputInfo , const InputInfoList ,
  const ArgList , const char *LinkingOutput) const {
@@ -4046,6 +4112,10 @@
   CmdArgs.push_back("-triple");
   CmdArgs.push_back(Args.MakeArgString(TripleStr));
 
+  if (const Arg *MJ = Args.getLastArg(options::OPT_MJ))
+DumpCompilationDatabase(C.getDriver(), MJ->getValue(), TripleStr, Output, Input, Args);
+  Args.ClaimAllArgs(options::OPT_MJ);
+
   if (IsCuda) {
 // We have to pass the triple of the host if compiling for a CUDA device and
 

[PATCH] D26979: Do not hard-code locale data in unit tests: get it from the OS instead

[Eric van Gyzen via Phabricator via cfe-commits]

vangyzen added inline comments.



Comment at: 
projects/libcxx/test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/grouping.pass.cpp:42
+assert(std::setlocale(LC_NUMERIC, LOCALE_en_US_UTF_8) != NULL);
+char *expected = strdup(std::localeconv()->grouping);
+assert(std::setlocale(LC_NUMERIC, "C") != NULL);

localeconv()->grouping can become invalid after we reset the locale to "C", so 
duplicate the string into a local.



Comment at: 
projects/libcxx/test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp:79
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == L',');
+assert(np.thousands_sep() == wexpected);
 }

This assertion now fails for me.  wexpected is 0xa0 (NBSP), which is correct.  
However, np.thousands_sep() is -62, which is 0xc2, which is the first byte of a 
UTF-8-encoded NBSP.  It looks like the library isn't correctly handling 
multibyte thousands separators.


https://reviews.llvm.org/D26979



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

[PATCH] D26979: Do not hard-code locale data in unit tests: get it from the OS instead

[Eric van Gyzen via Phabricator via cfe-commits]

vangyzen added a comment.

I'm glad you mentioned a multibyte thousands_sep, because it //is// multibyte 
in fr_FR.UTF-8 on FreeBSD 11.  Specifically, it's a no-break space (U+00A0).


https://reviews.llvm.org/D26979



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

[PATCH] D26979: Do not hard-code locale data in unit tests: get it from the OS instead

[Eric van Gyzen via Phabricator via cfe-commits]

vangyzen updated this revision to Diff 79322.
vangyzen added a comment.

Handle multibyte thousands_sep; do not reference possibly stale locale data


https://reviews.llvm.org/D26979

Files:
  
projects/libcxx/test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/grouping.pass.cpp
  
projects/libcxx/test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp

Index: projects/libcxx/test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp
===
--- projects/libcxx/test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp
+++ projects/libcxx/test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp
@@ -16,10 +16,8 @@
 
 // char_type thousands_sep() const;
 
-// TODO: investigation needed
-// XFAIL: linux-gnu
-
 #include 
+#include 
 #include 
 
 #include "platform_support.h" // locale name macros
@@ -40,29 +38,45 @@
 }
 }
 {
+char expected;
+wchar_t wexpected;
+assert(std::setlocale(LC_ALL, LOCALE_en_US_UTF_8) != NULL);
+const char *ts = std::localeconv()->thousands_sep;
+expected = *ts;
+int nb = mbtowc(, ts, strlen(ts)+1);
+assert(nb >= 0);
+assert(std::setlocale(LC_ALL, "C") != NULL);
 std::locale l(LOCALE_en_US_UTF_8);
 {
 typedef char C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == ',');
+assert(np.thousands_sep() == expected);
 }
 {
 typedef wchar_t C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == L',');
+assert(np.thousands_sep() == wexpected);
 }
 }
 {
+char expected;
+wchar_t wexpected;
+assert(std::setlocale(LC_ALL, LOCALE_fr_FR_UTF_8) != NULL);
+const char *ts = std::localeconv()->thousands_sep;
+expected = *ts;
+int nb = mbtowc(, ts, strlen(ts)+1);
+assert(nb >= 0);
+assert(std::setlocale(LC_ALL, "C") != NULL);
 std::locale l(LOCALE_fr_FR_UTF_8);
 {
 typedef char C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == ',');
+assert(np.thousands_sep() == expected);
 }
 {
 typedef wchar_t C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == L',');
+assert(np.thousands_sep() == wexpected);
 }
 }
 }
Index: projects/libcxx/test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/grouping.pass.cpp
===
--- projects/libcxx/test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/grouping.pass.cpp
+++ projects/libcxx/test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/grouping.pass.cpp
@@ -16,10 +16,8 @@
 
 // string grouping() const;
 
-// TODO: investigation needed
-// XFAIL: linux-gnu
-
 #include 
+#include 
 #include 
 
 #include "platform_support.h" // locale name macros
@@ -40,29 +38,37 @@
 }
 }
 {
+assert(std::setlocale(LC_NUMERIC, LOCALE_en_US_UTF_8) != NULL);
+char *expected = strdup(std::localeconv()->grouping);
+assert(std::setlocale(LC_NUMERIC, "C") != NULL);
 std::locale l(LOCALE_en_US_UTF_8);
 {
 typedef char C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.grouping() == "\3\3");
+assert(np.grouping() == expected);
 }
 {
 typedef wchar_t C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.grouping() == "\3\3");
+assert(np.grouping() == expected);
 }
+free(expected);
 }
 {
+assert(std::setlocale(LC_NUMERIC, LOCALE_fr_FR_UTF_8) != NULL);
+char *expected = strdup(std::localeconv()->grouping);
+assert(std::setlocale(LC_NUMERIC, "C") != NULL);
 std::locale l(LOCALE_fr_FR_UTF_8);
 {
 typedef char C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.grouping() == "\x7F");
+assert(np.grouping() == expected);
 }
 {
 typedef wchar_t C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.grouping() == "\x7F");
+assert(np.grouping() == expected);
 }
+free(expected);
 }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D27138: Extend CompilationDatabase by a field for the output filename

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg created this revision.
joerg added a reviewer: klimek.
joerg added a subscriber: cfe-commits.
joerg set the repository for this revision to rL LLVM.

In bigger projects like an Operating System, the same source code is often 
compiled in slightly different ways. This could be the difference between PIC 
and non-PIC code for static vs dynamic libraries, it could also be the 
difference between size optimised versions of tools for ramdisk images. At the 
moment, the compilation database has no way to distinguish such cases. As first 
step, add a field in the JSON format for it and process it accordingly.


Repository:
  rL LLVM

https://reviews.llvm.org/D27138

Files:
  docs/JSONCompilationDatabase.rst
  include/clang/Tooling/CompilationDatabase.h
  include/clang/Tooling/JSONCompilationDatabase.h
  lib/Tooling/CompilationDatabase.cpp
  lib/Tooling/JSONCompilationDatabase.cpp
  unittests/Tooling/CompilationDatabaseTest.cpp

Index: docs/JSONCompilationDatabase.rst
===
--- docs/JSONCompilationDatabase.rst
+++ docs/JSONCompilationDatabase.rst
@@ -80,6 +80,9 @@
supported.
 -  **arguments:** The compile command executed as list of strings.
Either **arguments** or **command** is required.
+-  **output:** The name of the output created by this compilation step.
+   This field is optional. It can be used to distinguish different processing
+   modes of the same input file.
 
 Build System Integration
 
Index: include/clang/Tooling/CompilationDatabase.h
===
--- include/clang/Tooling/CompilationDatabase.h
+++ include/clang/Tooling/CompilationDatabase.h
@@ -43,10 +43,11 @@
 struct CompileCommand {
   CompileCommand() {}
   CompileCommand(Twine Directory, Twine Filename,
- std::vector CommandLine)
+ std::vector CommandLine, Twine Output)
   : Directory(Directory.str()),
 Filename(Filename.str()),
-CommandLine(std::move(CommandLine)) {}
+CommandLine(std::move(CommandLine)),
+Output(Output.str()){}
 
   /// \brief The working directory the command was executed from.
   std::string Directory;
@@ -57,6 +58,9 @@
   /// \brief The command line that was executed.
   std::vector CommandLine;
 
+  /// The output file associated with the command.
+  std::string Output;
+
   /// \brief An optional mapping from each file's path to its content for all
   /// files needed for the compilation that are not available via the file
   /// system.
Index: include/clang/Tooling/JSONCompilationDatabase.h
===
--- include/clang/Tooling/JSONCompilationDatabase.h
+++ include/clang/Tooling/JSONCompilationDatabase.h
@@ -103,15 +103,17 @@
   /// failed.
   bool parse(std::string );
 
-  // Tuple (directory, filename, commandline) where 'commandline' points to the
-  // corresponding scalar nodes in the YAML stream.
+  // Tuple (directory, filename, commandline, output) where 'commandline'
+  // points to the corresponding scalar nodes in the YAML stream.
   // If the command line contains a single argument, it is a shell-escaped
   // command line.
   // Otherwise, each entry in the command line vector is a literal
   // argument to the compiler.
+  // The output field may be a nullptr.
   typedef std::tuple> CompileCommandRef;
+ std::vector,
+ llvm::yaml::ScalarNode *> CompileCommandRef;
 
   /// \brief Converts the given array of CompileCommandRefs to CompileCommands.
   void getCommands(ArrayRef CommandsRef,
Index: lib/Tooling/CompilationDatabase.cpp
===
--- lib/Tooling/CompilationDatabase.cpp
+++ lib/Tooling/CompilationDatabase.cpp
@@ -300,7 +300,8 @@
   ToolCommandLine.insert(ToolCommandLine.end(),
  CommandLine.begin(), CommandLine.end());
   CompileCommands.emplace_back(Directory, StringRef(),
-   std::move(ToolCommandLine));
+   std::move(ToolCommandLine),
+   StringRef());
 }
 
 std::vector
Index: lib/Tooling/JSONCompilationDatabase.cpp
===
--- lib/Tooling/JSONCompilationDatabase.cpp
+++ lib/Tooling/JSONCompilationDatabase.cpp
@@ -257,10 +257,13 @@
   for (int I = 0, E = CommandsRef.size(); I != E; ++I) {
 SmallString<8> DirectoryStorage;
 SmallString<32> FilenameStorage;
+SmallString<32> OutputStorage;
+auto Output = std::get<3>(CommandsRef[I]);
 Commands.emplace_back(
 std::get<0>(CommandsRef[I])->getValue(DirectoryStorage),
 std::get<1>(CommandsRef[I])->getValue(FilenameStorage),
-nodeToCommandLine(Syntax, std::get<2>(CommandsRef[I])));
+nodeToCommandLine(Syntax, std::get<2>(CommandsRef[I])),
+Output ? 

r287947 - Typo.

[Joerg Sonnenberger via cfe-commits]

Author: joerg
Date: Fri Nov 25 14:15:57 2016
New Revision: 287947

URL: http://llvm.org/viewvc/llvm-project?rev=287947=rev
Log:
Typo.

Modified:
cfe/trunk/lib/Tooling/ArgumentsAdjusters.cpp

Modified: cfe/trunk/lib/Tooling/ArgumentsAdjusters.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/ArgumentsAdjusters.cpp?rev=287947=287946=287947=diff
==
--- cfe/trunk/lib/Tooling/ArgumentsAdjusters.cpp (original)
+++ cfe/trunk/lib/Tooling/ArgumentsAdjusters.cpp Fri Nov 25 14:15:57 2016
@@ -17,7 +17,7 @@
 namespace clang {
 namespace tooling {
 
-/// Add -fsyntax-only option to the commnand line arguments.
+/// Add -fsyntax-only option to the command line arguments.
 ArgumentsAdjuster getClangSyntaxOnlyAdjuster() {
   return [](const CommandLineArguments , StringRef /*unused*/) {
 CommandLineArguments AdjustedArgs;


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

[PATCH] D27099: [OpenCL] Prohibit using reserve_id_t in program scope.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:5924
 
-  DeclContext *OriginalDC = DC;
-  bool IsLocalExternDecl = SC == SC_Extern &&
-   adjustContextForLocalExternDecl(DC);
+// OpenCL 1.2 spec, p6.9 r:
+// The event type cannot be used to declare a program scope variable.

The format is typically:
OpenCL v1.2 s6.9.r



Comment at: lib/Sema/SemaDecl.cpp:5926
+// The event type cannot be used to declare a program scope variable.
+// OpenCL v2.0 s6.9.q The clk_event_t and reserve_id_t types cannot be 
declared in program scope.
+if (NULL == S->getParent()) {

This line looks inconsistent with above! Put ":" after OpenCL v2.0 s6.9.q.

Also formatting seems off. At least the line width.


https://reviews.llvm.org/D27099



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

[PATCH] D27084: [OpenMP] Sema and parsing for 'teams distribute parallel for simd' pragma

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/Basic/OpenMPKinds.cpp:685
 
 bool clang::isOpenMPWorksharingDirective(OpenMPDirectiveKind DKind) {
   return DKind == OMPD_for || DKind == OMPD_for_simd ||

I think this new directive must be considered as a worksharing directive also.



Comment at: lib/Parse/ParseOpenMP.cpp:21
 #include "llvm/ADT/PointerIntPair.h"
-
 using namespace clang;

Restore this line



Comment at: test/OpenMP/nesting_of_regions.cpp:3326
   }
-#pragma omp ordered
   {

what about teams distribute parallel for simd inside the ordered directive? Why 
this one is removed?


https://reviews.llvm.org/D27084



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

[Phabricator] Emails are now sent via Phabricator

[Eric Liu via cfe-commits]

Hi all,

Emails from Phabricator are now sent with "senders" as:
 via Phabricator 

Previously, Phabricator sent emails on behalf of users using their email
addresses, which has caused DMARC failure for some mail service providers
(e.g. gmail, yahoo). Some of your emails might have been marked as spam due
to this. This is now fixed by always sending emails with Phabricator's
email address.

No action is required from users, but you might want to check your spam box
for any missing emails from Phabricator.

Regards,
Eric
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r287929 - Do not do raw name replacement when FromDecl is a class forward-declaration.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Fri Nov 25 10:02:49 2016
New Revision: 287929

URL: http://llvm.org/viewvc/llvm-project?rev=287929=rev
Log:
Do not do raw name replacement when FromDecl is a class forward-declaration.

Summary:
If the `FromDecl` is a class forward declaration, the reference is
still considered as referring to the original definition given the nature
of forward-declarations, so we can't do a raw name replacement in this case.

Reviewers: bkramer

Subscribers: cfe-commits, klimek

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

Modified:
cfe/trunk/lib/Tooling/Core/Lookup.cpp
cfe/trunk/unittests/Tooling/LookupTest.cpp

Modified: cfe/trunk/lib/Tooling/Core/Lookup.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Core/Lookup.cpp?rev=287929=287928=287929=diff
==
--- cfe/trunk/lib/Tooling/Core/Lookup.cpp (original)
+++ cfe/trunk/lib/Tooling/Core/Lookup.cpp Fri Nov 25 10:02:49 2016
@@ -13,6 +13,7 @@
 
 #include "clang/Tooling/Core/Lookup.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
 using namespace clang;
 using namespace clang::tooling;
 
@@ -121,14 +122,20 @@ std::string tooling::replaceNestedName(c
  "Expected fully-qualified name!");
 
   // We can do a raw name replacement when we are not inside the namespace for
-  // the original function and it is not in the global namespace.  The
+  // the original class/function and it is not in the global namespace.  The
   // assumption is that outside the original namespace we must have a using
   // statement that makes this work out and that other parts of this refactor
-  // will automatically fix using statements to point to the new function
+  // will automatically fix using statements to point to the new 
class/function.
+  // However, if the `FromDecl` is a class forward declaration, the reference 
is
+  // still considered as referring to the original definition, so we can't do a
+  // raw name replacement in this case.
   const bool class_name_only = !Use;
   const bool in_global_namespace =
   isa(FromDecl->getDeclContext());
-  if (class_name_only && !in_global_namespace &&
+  const bool is_class_forward_decl =
+  isa(FromDecl) &&
+  !cast(FromDecl)->isCompleteDefinition();
+  if (class_name_only && !in_global_namespace && !is_class_forward_decl &&
   !usingFromDifferentCanonicalNamespace(FromDecl->getDeclContext(),
 UseContext)) {
 auto Pos = ReplacementString.rfind("::");

Modified: cfe/trunk/unittests/Tooling/LookupTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/LookupTest.cpp?rev=287929=287928=287929=diff
==
--- cfe/trunk/unittests/Tooling/LookupTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/LookupTest.cpp Fri Nov 25 10:02:49 2016
@@ -13,7 +13,9 @@ using namespace clang;
 
 namespace {
 struct GetDeclsVisitor : TestVisitor {
-  std::function OnCall;
+  std::function OnCall = [&](CallExpr *Expr) {};
+  std::function OnRecordTypeLoc = [&](RecordTypeLoc Type) 
{
+  };
   SmallVector DeclStack;
 
   bool VisitCallExpr(CallExpr *Expr) {
@@ -21,6 +23,11 @@ struct GetDeclsVisitor : TestVisitor(Loc.getDecl());
+return tooling::replaceNestedName(
+nullptr, Visitor.DeclStack.back()->getDeclContext(), FD,
+ReplacementString);
+  };
+
+  Visitor.OnRecordTypeLoc = [&](RecordTypeLoc Type) {
+// Filter Types by name since there are other `RecordTypeLoc` in the test
+// file.
+if (Type.getDecl()->getQualifiedNameAsString() == "a::b::Foo")
+  EXPECT_EQ("x::Bar", replaceRecordTypeLoc(Type, "::a::x::Bar"));
+  };
+  Visitor.runOver("namespace a { namespace b {\n"
+  "class Foo;\n"
+  "namespace c { Foo f();; }\n"
+  "} }\n");
+
+  Visitor.OnRecordTypeLoc = [&](RecordTypeLoc Type) {
+// Filter Types by name since there are other `RecordTypeLoc` in the test
+// file.
+// `a::b::Foo` in using shadow decl is not `TypeLoc`.
+if (Type.getDecl()->getQualifiedNameAsString() == "a::b::Foo")
+  EXPECT_EQ("Bar", replaceRecordTypeLoc(Type, "::a::x::Bar"));
+  };
+  Visitor.runOver("namespace a { namespace b { class Foo {}; } }\n"
+  "namespace c { using a::b::Foo; Foo f();; }\n");
+}
+
 } // end anonymous namespace


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

[PATCH] D27132: Do not do raw name replacement when FromDecl is a class forward-declaration.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL287929: Do not do raw name replacement when FromDecl is a 
class forward-declaration. (authored by ioeric).

Changed prior to commit:
  https://reviews.llvm.org/D27132?vs=79307=79308#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D27132

Files:
  cfe/trunk/lib/Tooling/Core/Lookup.cpp
  cfe/trunk/unittests/Tooling/LookupTest.cpp

Index: cfe/trunk/unittests/Tooling/LookupTest.cpp
===
--- cfe/trunk/unittests/Tooling/LookupTest.cpp
+++ cfe/trunk/unittests/Tooling/LookupTest.cpp
@@ -13,23 +13,30 @@
 
 namespace {
 struct GetDeclsVisitor : TestVisitor {
-  std::function OnCall;
+  std::function OnCall = [&](CallExpr *Expr) {};
+  std::function OnRecordTypeLoc = [&](RecordTypeLoc Type) {
+  };
   SmallVector DeclStack;
 
   bool VisitCallExpr(CallExpr *Expr) {
 OnCall(Expr);
 return true;
   }
 
+  bool VisitRecordTypeLoc(RecordTypeLoc Loc) {
+OnRecordTypeLoc(Loc);
+return true;
+  }
+
   bool TraverseDecl(Decl *D) {
 DeclStack.push_back(D);
 bool Ret = TestVisitor::TraverseDecl(D);
 DeclStack.pop_back();
 return Ret;
   }
 };
 
-TEST(LookupTest, replaceNestedName) {
+TEST(LookupTest, replaceNestedFunctionName) {
   GetDeclsVisitor Visitor;
 
   auto replaceCallExpr = [&](const CallExpr *Expr,
@@ -121,4 +128,37 @@
   "} } }\n");
 }
 
+TEST(LookupTest, replaceNestedClassName) {
+  GetDeclsVisitor Visitor;
+
+  auto replaceRecordTypeLoc = [&](RecordTypeLoc Loc,
+  StringRef ReplacementString) {
+const auto *FD = cast(Loc.getDecl());
+return tooling::replaceNestedName(
+nullptr, Visitor.DeclStack.back()->getDeclContext(), FD,
+ReplacementString);
+  };
+
+  Visitor.OnRecordTypeLoc = [&](RecordTypeLoc Type) {
+// Filter Types by name since there are other `RecordTypeLoc` in the test
+// file.
+if (Type.getDecl()->getQualifiedNameAsString() == "a::b::Foo")
+  EXPECT_EQ("x::Bar", replaceRecordTypeLoc(Type, "::a::x::Bar"));
+  };
+  Visitor.runOver("namespace a { namespace b {\n"
+  "class Foo;\n"
+  "namespace c { Foo f();; }\n"
+  "} }\n");
+
+  Visitor.OnRecordTypeLoc = [&](RecordTypeLoc Type) {
+// Filter Types by name since there are other `RecordTypeLoc` in the test
+// file.
+// `a::b::Foo` in using shadow decl is not `TypeLoc`.
+if (Type.getDecl()->getQualifiedNameAsString() == "a::b::Foo")
+  EXPECT_EQ("Bar", replaceRecordTypeLoc(Type, "::a::x::Bar"));
+  };
+  Visitor.runOver("namespace a { namespace b { class Foo {}; } }\n"
+  "namespace c { using a::b::Foo; Foo f();; }\n");
+}
+
 } // end anonymous namespace
Index: cfe/trunk/lib/Tooling/Core/Lookup.cpp
===
--- cfe/trunk/lib/Tooling/Core/Lookup.cpp
+++ cfe/trunk/lib/Tooling/Core/Lookup.cpp
@@ -13,6 +13,7 @@
 
 #include "clang/Tooling/Core/Lookup.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
 using namespace clang;
 using namespace clang::tooling;
 
@@ -121,14 +122,20 @@
  "Expected fully-qualified name!");
 
   // We can do a raw name replacement when we are not inside the namespace for
-  // the original function and it is not in the global namespace.  The
+  // the original class/function and it is not in the global namespace.  The
   // assumption is that outside the original namespace we must have a using
   // statement that makes this work out and that other parts of this refactor
-  // will automatically fix using statements to point to the new function
+  // will automatically fix using statements to point to the new class/function.
+  // However, if the `FromDecl` is a class forward declaration, the reference is
+  // still considered as referring to the original definition, so we can't do a
+  // raw name replacement in this case.
   const bool class_name_only = !Use;
   const bool in_global_namespace =
   isa(FromDecl->getDeclContext());
-  if (class_name_only && !in_global_namespace &&
+  const bool is_class_forward_decl =
+  isa(FromDecl) &&
+  !cast(FromDecl)->isCompleteDefinition();
+  if (class_name_only && !in_global_namespace && !is_class_forward_decl &&
   !usingFromDifferentCanonicalNamespace(FromDecl->getDeclContext(),
 UseContext)) {
 auto Pos = ReplacementString.rfind("::");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27132: Do not do raw name replacement when FromDecl is a class forward-declaration.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 79307.
ioeric marked an inline comment as done.
ioeric added a comment.

- Address comments.


https://reviews.llvm.org/D27132

Files:
  lib/Tooling/Core/Lookup.cpp
  unittests/Tooling/LookupTest.cpp

Index: unittests/Tooling/LookupTest.cpp
===
--- unittests/Tooling/LookupTest.cpp
+++ unittests/Tooling/LookupTest.cpp
@@ -13,23 +13,30 @@
 
 namespace {
 struct GetDeclsVisitor : TestVisitor {
-  std::function OnCall;
+  std::function OnCall = [&](CallExpr *Expr) {};
+  std::function OnRecordTypeLoc = [&](RecordTypeLoc Type) {
+  };
   SmallVector DeclStack;
 
   bool VisitCallExpr(CallExpr *Expr) {
 OnCall(Expr);
 return true;
   }
 
+  bool VisitRecordTypeLoc(RecordTypeLoc Loc) {
+OnRecordTypeLoc(Loc);
+return true;
+  }
+
   bool TraverseDecl(Decl *D) {
 DeclStack.push_back(D);
 bool Ret = TestVisitor::TraverseDecl(D);
 DeclStack.pop_back();
 return Ret;
   }
 };
 
-TEST(LookupTest, replaceNestedName) {
+TEST(LookupTest, replaceNestedFunctionName) {
   GetDeclsVisitor Visitor;
 
   auto replaceCallExpr = [&](const CallExpr *Expr,
@@ -121,4 +128,37 @@
   "} } }\n");
 }
 
+TEST(LookupTest, replaceNestedClassName) {
+  GetDeclsVisitor Visitor;
+
+  auto replaceRecordTypeLoc = [&](RecordTypeLoc Loc,
+  StringRef ReplacementString) {
+const auto *FD = cast(Loc.getDecl());
+return tooling::replaceNestedName(
+nullptr, Visitor.DeclStack.back()->getDeclContext(), FD,
+ReplacementString);
+  };
+
+  Visitor.OnRecordTypeLoc = [&](RecordTypeLoc Type) {
+// Filter Types by name since there are other `RecordTypeLoc` in the test
+// file.
+if (Type.getDecl()->getQualifiedNameAsString() == "a::b::Foo")
+  EXPECT_EQ("x::Bar", replaceRecordTypeLoc(Type, "::a::x::Bar"));
+  };
+  Visitor.runOver("namespace a { namespace b {\n"
+  "class Foo;\n"
+  "namespace c { Foo f();; }\n"
+  "} }\n");
+
+  Visitor.OnRecordTypeLoc = [&](RecordTypeLoc Type) {
+// Filter Types by name since there are other `RecordTypeLoc` in the test
+// file.
+// `a::b::Foo` in using shadow decl is not `TypeLoc`.
+if (Type.getDecl()->getQualifiedNameAsString() == "a::b::Foo")
+  EXPECT_EQ("Bar", replaceRecordTypeLoc(Type, "::a::x::Bar"));
+  };
+  Visitor.runOver("namespace a { namespace b { class Foo {}; } }\n"
+  "namespace c { using a::b::Foo; Foo f();; }\n");
+}
+
 } // end anonymous namespace
Index: lib/Tooling/Core/Lookup.cpp
===
--- lib/Tooling/Core/Lookup.cpp
+++ lib/Tooling/Core/Lookup.cpp
@@ -13,6 +13,7 @@
 
 #include "clang/Tooling/Core/Lookup.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
 using namespace clang;
 using namespace clang::tooling;
 
@@ -121,14 +122,20 @@
  "Expected fully-qualified name!");
 
   // We can do a raw name replacement when we are not inside the namespace for
-  // the original function and it is not in the global namespace.  The
+  // the original class/function and it is not in the global namespace.  The
   // assumption is that outside the original namespace we must have a using
   // statement that makes this work out and that other parts of this refactor
-  // will automatically fix using statements to point to the new function
+  // will automatically fix using statements to point to the new class/function.
+  // However, if the `FromDecl` is a class forward declaration, the reference is
+  // still considered as referring to the original definition, so we can't do a
+  // raw name replacement in this case.
   const bool class_name_only = !Use;
   const bool in_global_namespace =
   isa(FromDecl->getDeclContext());
-  if (class_name_only && !in_global_namespace &&
+  const bool is_class_forward_decl =
+  isa(FromDecl) &&
+  !cast(FromDecl)->isCompleteDefinition();
+  if (class_name_only && !in_global_namespace && !is_class_forward_decl &&
   !usingFromDifferentCanonicalNamespace(FromDecl->getDeclContext(),
 UseContext)) {
 auto Pos = ReplacementString.rfind("::");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27132: Do not do raw name replacement when FromDecl is a class forward-declaration.

[Benjamin Kramer via Phabricator via cfe-commits]

bkramer accepted this revision.
bkramer added inline comments.
This revision is now accepted and ready to land.



Comment at: lib/Tooling/Core/Lookup.cpp:131
+  // still considered as referring to the original definition given the nature
+  // of forward-declarations, so we can't do a raw name replacement in this
+  // case.

the "given the nature" part doesn't add any useful information. Either drop it 
or explain in more detail what's the problem with forward decls.


https://reviews.llvm.org/D27132



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

[PATCH] D27132: Do not do raw name replacement when FromDecl is a class forward-declaration.

[Eric Liu via Phabricator via cfe-commits]

ioeric created this revision.
ioeric added a reviewer: bkramer.
ioeric added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

If the `FromDecl` is a class forward declaration, the reference is
still considered as referring to the original definition given the nature
of forward-declarations, so we can't do a raw name replacement in this case.


https://reviews.llvm.org/D27132

Files:
  lib/Tooling/Core/Lookup.cpp
  unittests/Tooling/LookupTest.cpp

Index: unittests/Tooling/LookupTest.cpp
===
--- unittests/Tooling/LookupTest.cpp
+++ unittests/Tooling/LookupTest.cpp
@@ -13,23 +13,30 @@
 
 namespace {
 struct GetDeclsVisitor : TestVisitor {
-  std::function OnCall;
+  std::function OnCall = [&](CallExpr *Expr) {};
+  std::function OnRecordTypeLoc = [&](RecordTypeLoc Type) {
+  };
   SmallVector DeclStack;
 
   bool VisitCallExpr(CallExpr *Expr) {
 OnCall(Expr);
 return true;
   }
 
+  bool VisitRecordTypeLoc(RecordTypeLoc Loc) {
+OnRecordTypeLoc(Loc);
+return true;
+  }
+
   bool TraverseDecl(Decl *D) {
 DeclStack.push_back(D);
 bool Ret = TestVisitor::TraverseDecl(D);
 DeclStack.pop_back();
 return Ret;
   }
 };
 
-TEST(LookupTest, replaceNestedName) {
+TEST(LookupTest, replaceNestedFunctionName) {
   GetDeclsVisitor Visitor;
 
   auto replaceCallExpr = [&](const CallExpr *Expr,
@@ -121,4 +128,37 @@
   "} } }\n");
 }
 
+TEST(LookupTest, replaceNestedClassName) {
+  GetDeclsVisitor Visitor;
+
+  auto replaceRecordTypeLoc = [&](RecordTypeLoc Loc,
+  StringRef ReplacementString) {
+const auto *FD = cast(Loc.getDecl());
+return tooling::replaceNestedName(
+nullptr, Visitor.DeclStack.back()->getDeclContext(), FD,
+ReplacementString);
+  };
+
+  Visitor.OnRecordTypeLoc = [&](RecordTypeLoc Type) {
+// Filter Types by name since there are other `RecordTypeLoc` in the test
+// file.
+if (Type.getDecl()->getQualifiedNameAsString() == "a::b::Foo")
+  EXPECT_EQ("x::Bar", replaceRecordTypeLoc(Type, "::a::x::Bar"));
+  };
+  Visitor.runOver("namespace a { namespace b {\n"
+  "class Foo;\n"
+  "namespace c { Foo f();; }\n"
+  "} }\n");
+
+  Visitor.OnRecordTypeLoc = [&](RecordTypeLoc Type) {
+// Filter Types by name since there are other `RecordTypeLoc` in the test
+// file.
+// `a::b::Foo` in using shadow decl is not `TypeLoc`.
+if (Type.getDecl()->getQualifiedNameAsString() == "a::b::Foo")
+  EXPECT_EQ("Bar", replaceRecordTypeLoc(Type, "::a::x::Bar"));
+  };
+  Visitor.runOver("namespace a { namespace b { class Foo {}; } }\n"
+  "namespace c { using a::b::Foo; Foo f();; }\n");
+}
+
 } // end anonymous namespace
Index: lib/Tooling/Core/Lookup.cpp
===
--- lib/Tooling/Core/Lookup.cpp
+++ lib/Tooling/Core/Lookup.cpp
@@ -13,6 +13,7 @@
 
 #include "clang/Tooling/Core/Lookup.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
 using namespace clang;
 using namespace clang::tooling;
 
@@ -121,14 +122,21 @@
  "Expected fully-qualified name!");
 
   // We can do a raw name replacement when we are not inside the namespace for
-  // the original function and it is not in the global namespace.  The
+  // the original class/function and it is not in the global namespace.  The
   // assumption is that outside the original namespace we must have a using
   // statement that makes this work out and that other parts of this refactor
-  // will automatically fix using statements to point to the new function
+  // will automatically fix using statements to point to the new class/function.
+  // However, if the `FromDecl` is a class forward declaration, the reference is
+  // still considered as referring to the original definition given the nature
+  // of forward-declarations, so we can't do a raw name replacement in this
+  // case.
   const bool class_name_only = !Use;
   const bool in_global_namespace =
   isa(FromDecl->getDeclContext());
-  if (class_name_only && !in_global_namespace &&
+  const bool is_class_forward_decl =
+  isa(FromDecl) &&
+  !cast(FromDecl)->isCompleteDefinition();
+  if (class_name_only && !in_global_namespace && !is_class_forward_decl &&
   !usingFromDifferentCanonicalNamespace(FromDecl->getDeclContext(),
 UseContext)) {
 auto Pos = ReplacementString.rfind("::");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r287926 - Document the arguments form of commands.

[Joerg Sonnenberger via cfe-commits]

Author: joerg
Date: Fri Nov 25 08:14:43 2016
New Revision: 287926

URL: http://llvm.org/viewvc/llvm-project?rev=287926=rev
Log:
Document the arguments form of commands.

Modified:
cfe/trunk/docs/JSONCompilationDatabase.rst

Modified: cfe/trunk/docs/JSONCompilationDatabase.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/JSONCompilationDatabase.rst?rev=287926=287925=287926=diff
==
--- cfe/trunk/docs/JSONCompilationDatabase.rst (original)
+++ cfe/trunk/docs/JSONCompilationDatabase.rst Fri Nov 25 08:14:43 2016
@@ -78,6 +78,8 @@ The contracts for each field in the comm
Parameters use shell quoting and shell escaping of quotes, with '``"``'
and '``\``' being the only special characters. Shell expansion is not
supported.
+-  **arguments:** The compile command executed as list of strings.
+   Either **arguments** or **command** is required.
 
 Build System Integration
 


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

[PATCH] D27125: Consider nested namespaces in the canonical namespace as canonical as well.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 79295.
ioeric marked an inline comment as done.
ioeric added a comment.

- Address review comment.


https://reviews.llvm.org/D27125

Files:
  lib/Tooling/Core/Lookup.cpp
  unittests/Tooling/LookupTest.cpp

Index: unittests/Tooling/LookupTest.cpp
===
--- unittests/Tooling/LookupTest.cpp
+++ unittests/Tooling/LookupTest.cpp
@@ -111,6 +111,14 @@
   "namespace a { namespace b { namespace {"
   "void f() { foo(); }"
   "} } }\n");
+
+  Visitor.OnCall = [&](CallExpr *Expr) {
+EXPECT_EQ("x::bar", replaceCallExpr(Expr, "::a::x::bar"));
+  };
+  Visitor.runOver("namespace a { namespace b { void foo(); } }\n"
+  "namespace a { namespace b { namespace c {"
+  "void f() { foo(); }"
+  "} } }\n");
 }
 
 } // end anonymous namespace
Index: lib/Tooling/Core/Lookup.cpp
===
--- lib/Tooling/Core/Lookup.cpp
+++ lib/Tooling/Core/Lookup.cpp
@@ -16,47 +16,65 @@
 using namespace clang;
 using namespace clang::tooling;
 
+// Gets all namespaces that \p Context is in as a vector (ignoring anonymous
+// namespaces). The inner namespaces come before outer namespaces in the vector.
+// For example, if the context is in the following namespace:
+//`namespace a { namespace b { namespace c ( ... ) } }`,
+// the vector will be `{c, b, a}`.
+static llvm::SmallVector
+getAllNamedNamespaces(const DeclContext *Context) {
+  llvm::SmallVector Namespaces;
+  auto GetNextNamedNamespace = [](const DeclContext *Context) {
+// Look past non-namespaces and anonymous namespaces on FromContext.
+while (Context && (!isa(Context) ||
+   cast(Context)->isAnonymousNamespace()))
+  Context = Context->getParent();
+return Context;
+  };
+  for (Context = GetNextNamedNamespace(Context); Context != nullptr;
+   Context = GetNextNamedNamespace(Context->getParent()))
+Namespaces.push_back(cast(Context));
+  return Namespaces;
+}
+
 // Returns true if the context in which the type is used and the context in
 // which the type is declared are the same semantical namespace but different
 // lexical namespaces.
 static bool
 usingFromDifferentCanonicalNamespace(const DeclContext *FromContext,
  const DeclContext *UseContext) {
-  while (true) {
-// Look past non-namespaces and anonymous namespaces on FromContext.
-// We can skip anonymous namespace because:
-// 1. `FromContext` and `UseContext` must be in the same anonymous
-// namespaces since referencing across anonymous namespaces is not possible.
-// 2. If `FromContext` and `UseContext` are in the same anonymous namespace,
-// the function will still return `false` as expected.
-while (FromContext &&
-   (!isa(FromContext) ||
-cast(FromContext)->isAnonymousNamespace()))
-  FromContext = FromContext->getParent();
-
-// Look past non-namespaces and anonymous namespaces on UseContext.
-while (UseContext &&
-   (!isa(UseContext) ||
-cast(UseContext)->isAnonymousNamespace()))
-  UseContext = UseContext->getParent();
-
-// We hit the root, no namespace collision.
-if (!FromContext || !UseContext)
-  return false;
-
+  // We can skip anonymous namespace because:
+  // 1. `FromContext` and `UseContext` must be in the same anonymous namespaces
+  // since referencing across anonymous namespaces is not possible.
+  // 2. If `FromContext` and `UseContext` are in the same anonymous namespace,
+  // the function will still return `false` as expected.
+  llvm::SmallVector FromNamespaces =
+  getAllNamedNamespaces(FromContext);
+  llvm::SmallVector UseNamespaces =
+  getAllNamedNamespaces(UseContext);
+  // If `UseContext` has fewer level of nested namespaces, it cannot be in the
+  // same canonical namespace as the `FromContext`.
+  if (UseNamespaces.size() < FromNamespaces.size())
+return false;
+  unsigned Diff = UseNamespaces.size() - FromNamespaces.size();
+  auto FromIter = FromNamespaces.begin();
+  // Only compare `FromNamespaces` with namespaces in `UseNamespaces` that can
+  // collide, i.e. the top N namespaces where N is the number of namespaces in
+  // `FromNamespaces`.
+  auto UseIter = UseNamespaces.begin() + Diff;
+  for (; FromIter != FromNamespaces.end() && UseIter != UseNamespaces.end();
+   ++FromIter, ++UseIter) {
 // Literally the same namespace, not a collision.
-if (FromContext == UseContext)
+if (*FromIter == *UseIter)
   return false;
-
-// Now check the names. If they match we have a different namespace with the
-// same name.
-if (cast(FromContext)->getDeclName() ==
-cast(UseContext)->getDeclName())
+// Now check the names. If they match we have a different canonical
+// namespace with the same 

[PATCH] D27125: Consider nested namespaces in the canonical namespace as canonical as well.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL287924: Consider nested namespaces in the canonical 
namespace as canonical as well. (authored by ioeric).

Changed prior to commit:
  https://reviews.llvm.org/D27125?vs=79295=79296#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D27125

Files:
  cfe/trunk/lib/Tooling/Core/Lookup.cpp
  cfe/trunk/unittests/Tooling/LookupTest.cpp

Index: cfe/trunk/lib/Tooling/Core/Lookup.cpp
===
--- cfe/trunk/lib/Tooling/Core/Lookup.cpp
+++ cfe/trunk/lib/Tooling/Core/Lookup.cpp
@@ -16,47 +16,65 @@
 using namespace clang;
 using namespace clang::tooling;
 
+// Gets all namespaces that \p Context is in as a vector (ignoring anonymous
+// namespaces). The inner namespaces come before outer namespaces in the vector.
+// For example, if the context is in the following namespace:
+//`namespace a { namespace b { namespace c ( ... ) } }`,
+// the vector will be `{c, b, a}`.
+static llvm::SmallVector
+getAllNamedNamespaces(const DeclContext *Context) {
+  llvm::SmallVector Namespaces;
+  auto GetNextNamedNamespace = [](const DeclContext *Context) {
+// Look past non-namespaces and anonymous namespaces on FromContext.
+while (Context && (!isa(Context) ||
+   cast(Context)->isAnonymousNamespace()))
+  Context = Context->getParent();
+return Context;
+  };
+  for (Context = GetNextNamedNamespace(Context); Context != nullptr;
+   Context = GetNextNamedNamespace(Context->getParent()))
+Namespaces.push_back(cast(Context));
+  return Namespaces;
+}
+
 // Returns true if the context in which the type is used and the context in
 // which the type is declared are the same semantical namespace but different
 // lexical namespaces.
 static bool
 usingFromDifferentCanonicalNamespace(const DeclContext *FromContext,
  const DeclContext *UseContext) {
-  while (true) {
-// Look past non-namespaces and anonymous namespaces on FromContext.
-// We can skip anonymous namespace because:
-// 1. `FromContext` and `UseContext` must be in the same anonymous
-// namespaces since referencing across anonymous namespaces is not possible.
-// 2. If `FromContext` and `UseContext` are in the same anonymous namespace,
-// the function will still return `false` as expected.
-while (FromContext &&
-   (!isa(FromContext) ||
-cast(FromContext)->isAnonymousNamespace()))
-  FromContext = FromContext->getParent();
-
-// Look past non-namespaces and anonymous namespaces on UseContext.
-while (UseContext &&
-   (!isa(UseContext) ||
-cast(UseContext)->isAnonymousNamespace()))
-  UseContext = UseContext->getParent();
-
-// We hit the root, no namespace collision.
-if (!FromContext || !UseContext)
-  return false;
-
+  // We can skip anonymous namespace because:
+  // 1. `FromContext` and `UseContext` must be in the same anonymous namespaces
+  // since referencing across anonymous namespaces is not possible.
+  // 2. If `FromContext` and `UseContext` are in the same anonymous namespace,
+  // the function will still return `false` as expected.
+  llvm::SmallVector FromNamespaces =
+  getAllNamedNamespaces(FromContext);
+  llvm::SmallVector UseNamespaces =
+  getAllNamedNamespaces(UseContext);
+  // If `UseContext` has fewer level of nested namespaces, it cannot be in the
+  // same canonical namespace as the `FromContext`.
+  if (UseNamespaces.size() < FromNamespaces.size())
+return false;
+  unsigned Diff = UseNamespaces.size() - FromNamespaces.size();
+  auto FromIter = FromNamespaces.begin();
+  // Only compare `FromNamespaces` with namespaces in `UseNamespaces` that can
+  // collide, i.e. the top N namespaces where N is the number of namespaces in
+  // `FromNamespaces`.
+  auto UseIter = UseNamespaces.begin() + Diff;
+  for (; FromIter != FromNamespaces.end() && UseIter != UseNamespaces.end();
+   ++FromIter, ++UseIter) {
 // Literally the same namespace, not a collision.
-if (FromContext == UseContext)
+if (*FromIter == *UseIter)
   return false;
-
-// Now check the names. If they match we have a different namespace with the
-// same name.
-if (cast(FromContext)->getDeclName() ==
-cast(UseContext)->getDeclName())
+// Now check the names. If they match we have a different canonical
+// namespace with the same name.
+if (cast(*FromIter)->getDeclName() ==
+cast(*UseIter)->getDeclName())
   return true;
-
-FromContext = FromContext->getParent();
-UseContext = UseContext->getParent();
   }
+  assert(FromIter == FromNamespaces.end() && UseIter == UseNamespaces.end());
+  return false;
 }
 
 static StringRef getBestNamespaceSubstr(const DeclContext *DeclA,
Index: cfe/trunk/unittests/Tooling/LookupTest.cpp

[PATCH] D27099: [OpenCL] Prohibit using reserve_id_t in program scope.

[Egor Churaev via Phabricator via cfe-commits]

echuraev updated this revision to Diff 79292.
echuraev marked 3 inline comments as done.

https://reviews.llvm.org/D27099

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDecl.cpp
  test/SemaOpenCL/event_t.cl
  test/SemaOpenCL/invalid-clk-events-cl2.0.cl
  test/SemaOpenCL/invalid-pipes-cl2.0.cl

Index: test/SemaOpenCL/invalid-pipes-cl2.0.cl
===
--- test/SemaOpenCL/invalid-pipes-cl2.0.cl
+++ test/SemaOpenCL/invalid-pipes-cl2.0.cl
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
 
+global pipe int gp;// expected-error {{type '__global read_only pipe int' can only be used as a function parameter in OpenCL}}
+global reserve_id_t rid;  // expected-error {{the '__global reserve_id_t' type cannot be used to declare a program scope variable}}
+
 void test1(pipe int *p) {// expected-error {{pipes packet types cannot be of reference type}}
 }
 void test2(pipe p) {// expected-error {{missing actual type specifier for pipe}}
@@ -20,3 +23,8 @@
 
 typedef pipe int pipe_int_t;
 pipe_int_t test6() {} // expected-error{{declaring function return value of type 'pipe_int_t' (aka 'read_only pipe int') is not allowed}}
+
+bool test_id_comprision(void) {
+  reserve_id_t id1, id2;
+  return (id1 == id2);  // expected-error {{invalid operands to binary expression ('reserve_id_t' and 'reserve_id_t')}}
+}
Index: test/SemaOpenCL/invalid-clk-events-cl2.0.cl
===
--- /dev/null
+++ test/SemaOpenCL/invalid-clk-events-cl2.0.cl
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
+
+global clk_event_t ce; // expected-error {{the '__global clk_event_t' type cannot be used to declare a program scope variable}}
Index: test/SemaOpenCL/event_t.cl
===
--- test/SemaOpenCL/event_t.cl
+++ test/SemaOpenCL/event_t.cl
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
 
-event_t glb_evt; // expected-error {{the event_t type cannot be used to declare a program scope variable}}
+event_t glb_evt; // expected-error {{the 'event_t' type cannot be used to declare a program scope variable}}
 
 constant struct evt_s {
   event_t evt; // expected-error {{the 'event_t' type cannot be used to declare a structure or union field}}
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -5909,32 +5909,30 @@
 return nullptr;
   }
 
-  // OpenCL v2.0 s6.9.b - Image type can only be used as a function argument.
-  // OpenCL v2.0 s6.13.16.1 - Pipe type can only be used as a function
-  // argument.
-  if (getLangOpts().OpenCL && (R->isImageType() || R->isPipeType())) {
-Diag(D.getIdentifierLoc(),
- diag::err_opencl_type_can_only_be_used_as_function_parameter)
-<< R;
-D.setInvalidType();
-return nullptr;
-  }
-
-  DeclSpec::SCS SCSpec = D.getDeclSpec().getStorageClassSpec();
-  StorageClass SC = StorageClassSpecToVarDeclStorageClass(D.getDeclSpec());
-
-  // dllimport globals without explicit storage class are treated as extern. We
-  // have to change the storage class this early to get the right DeclContext.
-  if (SC == SC_None && !DC->isRecord() &&
-  hasParsedAttr(S, D, AttributeList::AT_DLLImport) &&
-  !hasParsedAttr(S, D, AttributeList::AT_DLLExport))
-SC = SC_Extern;
+  if (getLangOpts().OpenCL) {
+// OpenCL v2.0 s6.9.b - Image type can only be used as a function argument.
+// OpenCL v2.0 s6.13.16.1 - Pipe type can only be used as a function
+// argument.
+if (R->isImageType() || R->isPipeType()) {
+  Diag(D.getIdentifierLoc(),
+   diag::err_opencl_type_can_only_be_used_as_function_parameter)
+  << R;
+  D.setInvalidType();
+  return nullptr;
+}
 
-  DeclContext *OriginalDC = DC;
-  bool IsLocalExternDecl = SC == SC_Extern &&
-   adjustContextForLocalExternDecl(DC);
+// OpenCL 1.2 spec, p6.9 r:
+// The event type cannot be used to declare a program scope variable.
+// OpenCL v2.0 s6.9.q The clk_event_t and reserve_id_t types cannot be declared in program scope.
+if (NULL == S->getParent()) {
+  if (R->isReserveIDT() || R->isClkEventT() || R->isEventT()) {
+Diag(D.getIdentifierLoc(),
+ diag::err_invalid_type_for_program_scope_var) << R;
+D.setInvalidType();
+return nullptr;
+  }
+}
 
-  if (getLangOpts().OpenCL) {
 // OpenCL v1.0 s6.8.a.3: Pointers to functions are not allowed.
 QualType NR = R;
 while (NR->isPointerType()) {
@@ -5954,8 +5952,40 @@
 D.setInvalidType();
   }
 }
+
+// OpenCL v1.2 s6.9.b p4:
+// The sampler type cannot be used with the __local and __global address
+// space qualifiers.
+if (R->isSamplerT() && 

r287924 - Consider nested namespaces in the canonical namespace as canonical as well.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Fri Nov 25 06:39:03 2016
New Revision: 287924

URL: http://llvm.org/viewvc/llvm-project?rev=287924=rev
Log:
Consider nested namespaces in the canonical namespace as canonical as well.

Summary:
For example, this case was missed when looking for different but canonical
namespaces. UseContext in this case should be considered as in the canonical
namespace.
```
namespace a { namespace b {  } }
namespace a { namespace b { namespace c {  } } }
```
Added some commenting.

Reviewers: bkramer

Subscribers: klimek, cfe-commits

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

Modified:
cfe/trunk/lib/Tooling/Core/Lookup.cpp
cfe/trunk/unittests/Tooling/LookupTest.cpp

Modified: cfe/trunk/lib/Tooling/Core/Lookup.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Core/Lookup.cpp?rev=287924=287923=287924=diff
==
--- cfe/trunk/lib/Tooling/Core/Lookup.cpp (original)
+++ cfe/trunk/lib/Tooling/Core/Lookup.cpp Fri Nov 25 06:39:03 2016
@@ -16,47 +16,65 @@
 using namespace clang;
 using namespace clang::tooling;
 
+// Gets all namespaces that \p Context is in as a vector (ignoring anonymous
+// namespaces). The inner namespaces come before outer namespaces in the 
vector.
+// For example, if the context is in the following namespace:
+//`namespace a { namespace b { namespace c ( ... ) } }`,
+// the vector will be `{c, b, a}`.
+static llvm::SmallVector
+getAllNamedNamespaces(const DeclContext *Context) {
+  llvm::SmallVector Namespaces;
+  auto GetNextNamedNamespace = [](const DeclContext *Context) {
+// Look past non-namespaces and anonymous namespaces on FromContext.
+while (Context && (!isa(Context) ||
+   cast(Context)->isAnonymousNamespace()))
+  Context = Context->getParent();
+return Context;
+  };
+  for (Context = GetNextNamedNamespace(Context); Context != nullptr;
+   Context = GetNextNamedNamespace(Context->getParent()))
+Namespaces.push_back(cast(Context));
+  return Namespaces;
+}
+
 // Returns true if the context in which the type is used and the context in
 // which the type is declared are the same semantical namespace but different
 // lexical namespaces.
 static bool
 usingFromDifferentCanonicalNamespace(const DeclContext *FromContext,
  const DeclContext *UseContext) {
-  while (true) {
-// Look past non-namespaces and anonymous namespaces on FromContext.
-// We can skip anonymous namespace because:
-// 1. `FromContext` and `UseContext` must be in the same anonymous
-// namespaces since referencing across anonymous namespaces is not 
possible.
-// 2. If `FromContext` and `UseContext` are in the same anonymous 
namespace,
-// the function will still return `false` as expected.
-while (FromContext &&
-   (!isa(FromContext) ||
-cast(FromContext)->isAnonymousNamespace()))
-  FromContext = FromContext->getParent();
-
-// Look past non-namespaces and anonymous namespaces on UseContext.
-while (UseContext &&
-   (!isa(UseContext) ||
-cast(UseContext)->isAnonymousNamespace()))
-  UseContext = UseContext->getParent();
-
-// We hit the root, no namespace collision.
-if (!FromContext || !UseContext)
-  return false;
-
+  // We can skip anonymous namespace because:
+  // 1. `FromContext` and `UseContext` must be in the same anonymous namespaces
+  // since referencing across anonymous namespaces is not possible.
+  // 2. If `FromContext` and `UseContext` are in the same anonymous namespace,
+  // the function will still return `false` as expected.
+  llvm::SmallVector FromNamespaces =
+  getAllNamedNamespaces(FromContext);
+  llvm::SmallVector UseNamespaces =
+  getAllNamedNamespaces(UseContext);
+  // If `UseContext` has fewer level of nested namespaces, it cannot be in the
+  // same canonical namespace as the `FromContext`.
+  if (UseNamespaces.size() < FromNamespaces.size())
+return false;
+  unsigned Diff = UseNamespaces.size() - FromNamespaces.size();
+  auto FromIter = FromNamespaces.begin();
+  // Only compare `FromNamespaces` with namespaces in `UseNamespaces` that can
+  // collide, i.e. the top N namespaces where N is the number of namespaces in
+  // `FromNamespaces`.
+  auto UseIter = UseNamespaces.begin() + Diff;
+  for (; FromIter != FromNamespaces.end() && UseIter != UseNamespaces.end();
+   ++FromIter, ++UseIter) {
 // Literally the same namespace, not a collision.
-if (FromContext == UseContext)
+if (*FromIter == *UseIter)
   return false;
-
-// Now check the names. If they match we have a different namespace with 
the
-// same name.
-if (cast(FromContext)->getDeclName() ==
-cast(UseContext)->getDeclName())
+// Now check the names. If they match we have a different canonical
+// namespace with the same name.
+if 

[PATCH] D27125: Consider nested namespaces in the canonical namespace as canonical as well.

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

Testing sender again


https://reviews.llvm.org/D27125



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

[PATCH] D27125: Consider nested namespaces in the canonical namespace as canonical as well.

[Eric Liu via cfe-commits]

ioeric added a comment.

Test email sender.


https://reviews.llvm.org/D27125



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

[PATCH] D27125: Consider nested namespaces in the canonical namespace as canonical as well.

[Benjamin Kramer via cfe-commits]

bkramer added inline comments.



Comment at: lib/Tooling/Core/Lookup.cpp:27
+  llvm::SmallVector Namespaces;
+  auto GetNextNameNamespace = [](const DeclContext *Context) {
+// Look past non-namespaces and anonymous namespaces on FromContext.

GetNextName**d**Namespace


https://reviews.llvm.org/D27125



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

[PATCH] D27125: Consider nested namespaces in the canonical namespace as canonical as well.

[Eric Liu via cfe-commits]

ioeric created this revision.
ioeric added a reviewer: bkramer.
ioeric added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

For example, this case was missed when looking for different but canonical
namespaces. UseContext in this case should be considered as in the canonical
namespace.

  namespace a { namespace b {  } }
  namespace a { namespace b { namespace c {  } } }

Added some commenting.


https://reviews.llvm.org/D27125

Files:
  lib/Tooling/Core/Lookup.cpp
  unittests/Tooling/LookupTest.cpp

Index: unittests/Tooling/LookupTest.cpp
===
--- unittests/Tooling/LookupTest.cpp
+++ unittests/Tooling/LookupTest.cpp
@@ -111,6 +111,14 @@
   "namespace a { namespace b { namespace {"
   "void f() { foo(); }"
   "} } }\n");
+
+  Visitor.OnCall = [&](CallExpr *Expr) {
+EXPECT_EQ("x::bar", replaceCallExpr(Expr, "::a::x::bar"));
+  };
+  Visitor.runOver("namespace a { namespace b { void foo(); } }\n"
+  "namespace a { namespace b { namespace c {"
+  "void f() { foo(); }"
+  "} } }\n");
 }
 
 } // end anonymous namespace
Index: lib/Tooling/Core/Lookup.cpp
===
--- lib/Tooling/Core/Lookup.cpp
+++ lib/Tooling/Core/Lookup.cpp
@@ -16,47 +16,66 @@
 using namespace clang;
 using namespace clang::tooling;
 
+// Gets all namespaces that \p Context is in as a vector (ignoring anonymous
+// namespaces). The inner namespaces come before outer namespaces in the vector.
+// For example, if the context is in the following namespace:
+//`namespace a { namespace b { namespace c ( ... ) } }`,
+// the vector will be `{c, b, a}`.
+static llvm::SmallVector
+getAllNamedNamespaces(const DeclContext *Context) {
+  llvm::SmallVector Namespaces;
+  auto GetNextNameNamespace = [](const DeclContext *Context) {
+// Look past non-namespaces and anonymous namespaces on FromContext.
+while (Context &&
+   (!isa(Context) ||
+cast(Context)->isAnonymousNamespace()))
+  Context = Context->getParent();
+return Context;
+  };
+  for (Context = GetNextNameNamespace(Context); Context != nullptr;
+   Context = GetNextNameNamespace(Context->getParent()))
+Namespaces.push_back(cast(Context));
+  return Namespaces;
+}
+
 // Returns true if the context in which the type is used and the context in
 // which the type is declared are the same semantical namespace but different
 // lexical namespaces.
 static bool
 usingFromDifferentCanonicalNamespace(const DeclContext *FromContext,
  const DeclContext *UseContext) {
-  while (true) {
-// Look past non-namespaces and anonymous namespaces on FromContext.
-// We can skip anonymous namespace because:
-// 1. `FromContext` and `UseContext` must be in the same anonymous
-// namespaces since referencing across anonymous namespaces is not possible.
-// 2. If `FromContext` and `UseContext` are in the same anonymous namespace,
-// the function will still return `false` as expected.
-while (FromContext &&
-   (!isa(FromContext) ||
-cast(FromContext)->isAnonymousNamespace()))
-  FromContext = FromContext->getParent();
-
-// Look past non-namespaces and anonymous namespaces on UseContext.
-while (UseContext &&
-   (!isa(UseContext) ||
-cast(UseContext)->isAnonymousNamespace()))
-  UseContext = UseContext->getParent();
-
-// We hit the root, no namespace collision.
-if (!FromContext || !UseContext)
-  return false;
-
+  // We can skip anonymous namespace because:
+  // 1. `FromContext` and `UseContext` must be in the same anonymous namespaces
+  // since referencing across anonymous namespaces is not possible.
+  // 2. If `FromContext` and `UseContext` are in the same anonymous namespace,
+  // the function will still return `false` as expected.
+  llvm::SmallVector FromNamespaces =
+  getAllNamedNamespaces(FromContext);
+  llvm::SmallVector UseNamespaces =
+  getAllNamedNamespaces(UseContext);
+  // If `UseContext` has fewer level of nested namespaces, it cannot be in the
+  // same canonical namespace as the `FromContext`.
+  if (UseNamespaces.size() < FromNamespaces.size())
+return false;
+  unsigned Diff = UseNamespaces.size() - FromNamespaces.size();
+  auto FromIter = FromNamespaces.begin();
+  // Only compare `FromNamespaces` with namespaces in `UseNamespaces` that can
+  // collide, i.e. the top N namespaces where N is the number of namespaces in
+  // `FromNamespaces`.
+  auto UseIter = UseNamespaces.begin() + Diff;
+  for (; FromIter != FromNamespaces.end() && UseIter != UseNamespaces.end();
+   ++FromIter, ++UseIter) {
 // Literally the same namespace, not a collision.
-if (FromContext == UseContext)
+if (*FromIter == *UseIter)
   return false;
-
-// Now check