I suspect it was caused by the new line splitting ("\r\n" vs "\n"?) on
Windows. In the test, I write some strings (expected to be new line
delimited) to a temp file (
https://github.com/llvm-mirror/clang-tools-extra/blob/master/test/change-namespace/white-list.cpp#L1),
read the content, and then split with "\n" (
https://github.com/llvm-mirror/clang-tools-extra/blob/master/change-namespace/tool/ClangChangeNamespace.cpp#L92).
I am trying trimming the lines to see if this fix the issue (r296458).Unfortunately, I don't have a Windows machine available for testing so I couldn't verify this locally. - Eric On Tue, Feb 28, 2017 at 10:28 AM Eric Liu <ioe...@google.com> wrote: > Sorry for losing track of this. > > I thought I fixed it with r296113. Not sure why windows builds are still > failing. I'll look into this. > > Thanks, > Eric > > On Tue, Feb 28, 2017 at 2:19 AM Galina Kistanova <gkistan...@gmail.com> > wrote: > > Hello Eric, > > This commit broke test on one of our builders: > > Clang Tools :: change-namespace/white-list.cpp > > > http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/6153/steps/test/logs/stdio > > Please have a look. > > Thanks > > > Galina > > > On Fri, Feb 24, 2017 at 3:54 AM, Eric Liu via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > > Author: ioeric > Date: Fri Feb 24 05:54:45 2017 > New Revision: 296110 > > URL: http://llvm.org/viewvc/llvm-project?rev=296110&view=rev > Log: > [change-namepsace] make it possible to whitelist symbols so they don't get > updated. > > Reviewers: hokein > > Reviewed By: hokein > > Subscribers: cfe-commits > > Differential Revision: https://reviews.llvm.org/D30328 > > Added: > clang-tools-extra/trunk/test/change-namespace/Inputs/ > clang-tools-extra/trunk/test/change-namespace/Inputs/fake-std.h > clang-tools-extra/trunk/test/change-namespace/white-list.cpp > Modified: > clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp > clang-tools-extra/trunk/change-namespace/ChangeNamespace.h > clang-tools-extra/trunk/change-namespace/tool/ClangChangeNamespace.cpp > > clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp > > Modified: clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp > URL: > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp?rev=296110&r1=296109&r2=296110&view=diff > > ============================================================================== > --- clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp (original) > +++ clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp Fri Feb > 24 05:54:45 2017 > @@ -290,6 +290,7 @@ AST_MATCHER(EnumDecl, isScoped) { > > ChangeNamespaceTool::ChangeNamespaceTool( > llvm::StringRef OldNs, llvm::StringRef NewNs, llvm::StringRef > FilePattern, > + llvm::ArrayRef<std::string> WhiteListedSymbolPatterns, > std::map<std::string, tooling::Replacements> *FileToReplacements, > llvm::StringRef FallbackStyle) > : FallbackStyle(FallbackStyle), > FileToReplacements(*FileToReplacements), > @@ -308,6 +309,9 @@ ChangeNamespaceTool::ChangeNamespaceTool > } > DiffOldNamespace = joinNamespaces(OldNsSplitted); > DiffNewNamespace = joinNamespaces(NewNsSplitted); > + > + for (const auto &Pattern : WhiteListedSymbolPatterns) > + WhiteListedSymbolRegexes.emplace_back(Pattern); > } > > void ChangeNamespaceTool::registerMatchers(ast_matchers::MatchFinder > *Finder) { > @@ -736,6 +740,9 @@ void ChangeNamespaceTool::replaceQualifi > Result.SourceManager->getSpellingLoc(End)), > *Result.SourceManager, Result.Context->getLangOpts()); > std::string FromDeclName = FromDecl->getQualifiedNameAsString(); > + for (llvm::Regex &RE : WhiteListedSymbolRegexes) > + if (RE.match(FromDeclName)) > + return; > std::string ReplaceName = > getShortestQualifiedNameInNamespace(FromDeclName, NewNs); > // Checks if there is any using namespace declarations that can shorten > the > > Modified: clang-tools-extra/trunk/change-namespace/ChangeNamespace.h > URL: > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/change-namespace/ChangeNamespace.h?rev=296110&r1=296109&r2=296110&view=diff > > ============================================================================== > --- clang-tools-extra/trunk/change-namespace/ChangeNamespace.h (original) > +++ clang-tools-extra/trunk/change-namespace/ChangeNamespace.h Fri Feb 24 > 05:54:45 2017 > @@ -50,6 +50,7 @@ public: > // files matching `FilePattern`. > ChangeNamespaceTool( > llvm::StringRef OldNs, llvm::StringRef NewNs, llvm::StringRef > FilePattern, > + llvm::ArrayRef<std::string> WhiteListedSymbolPatterns, > std::map<std::string, tooling::Replacements> *FileToReplacements, > llvm::StringRef FallbackStyle = "LLVM"); > > @@ -164,6 +165,9 @@ private: > // CallExpr and one as DeclRefExpr), we record all DeclRefExpr's that > have > // been processed so that we don't handle them twice. > llvm::SmallPtrSet<const clang::DeclRefExpr*, 16> ProcessedFuncRefs; > + // Patterns of symbol names whose references are not expected to be > updated > + // when changing namespaces around them. > + std::vector<llvm::Regex> WhiteListedSymbolRegexes; > }; > > } // namespace change_namespace > > Modified: > clang-tools-extra/trunk/change-namespace/tool/ClangChangeNamespace.cpp > URL: > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/change-namespace/tool/ClangChangeNamespace.cpp?rev=296110&r1=296109&r2=296110&view=diff > > ============================================================================== > --- clang-tools-extra/trunk/change-namespace/tool/ClangChangeNamespace.cpp > (original) > +++ clang-tools-extra/trunk/change-namespace/tool/ClangChangeNamespace.cpp > Fri Feb 24 05:54:45 2017 > @@ -73,6 +73,25 @@ cl::opt<std::string> Style("style", > cl::desc("The style name used for > reformatting."), > cl::init("LLVM"), > cl::cat(ChangeNamespaceCategory)); > > +cl::opt<std::string> WhiteListFile( > + "whitelist_file", > + cl::desc("A file containing regexes of symbol names that are not > expected " > + "to be updated when changing namespaces around them."), > + cl::init(""), cl::cat(ChangeNamespaceCategory)); > + > +llvm::ErrorOr<std::vector<std::string>> GetWhiteListedSymbolPatterns() { > + llvm::SmallVector<StringRef, 8> Lines; > + if (!WhiteListFile.empty()) { > + llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> File = > + llvm::MemoryBuffer::getFile(WhiteListFile); > + if (!File) > + return File.getError(); > + llvm::StringRef Content = File.get()->getBuffer(); > + Content.split(Lines, '\n', /*MaxSplit=*/-1, /*KeepEmpty=*/false); > + } > + return std::vector<std::string>(Lines.begin(), Lines.end()); > +} > + > } // anonymous namespace > > int main(int argc, const char **argv) { > @@ -81,8 +100,16 @@ int main(int argc, const char **argv) { > ChangeNamespaceCategory); > const auto &Files = OptionsParser.getSourcePathList(); > tooling::RefactoringTool Tool(OptionsParser.getCompilations(), Files); > + llvm::ErrorOr<std::vector<std::string>> WhiteListPatterns = > + GetWhiteListedSymbolPatterns(); > + if (!WhiteListPatterns) { > + llvm::errs() << "Failed to open whitelist file " << WhiteListFile << > ". " > + << WhiteListPatterns.getError().message() << "\n"; > + return 1; > + } > change_namespace::ChangeNamespaceTool NamespaceTool( > - OldNamespace, NewNamespace, FilePattern, &Tool.getReplacements(), > Style); > + OldNamespace, NewNamespace, FilePattern, *WhiteListPatterns, > + &Tool.getReplacements(), Style); > ast_matchers::MatchFinder Finder; > NamespaceTool.registerMatchers(&Finder); > std::unique_ptr<tooling::FrontendActionFactory> Factory = > > Added: clang-tools-extra/trunk/test/change-namespace/Inputs/fake-std.h > URL: > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/change-namespace/Inputs/fake-std.h?rev=296110&view=auto > > ============================================================================== > --- clang-tools-extra/trunk/test/change-namespace/Inputs/fake-std.h (added) > +++ clang-tools-extra/trunk/test/change-namespace/Inputs/fake-std.h Fri > Feb 24 05:54:45 2017 > @@ -0,0 +1,5 @@ > +namespace std { > + class STD {}; > +} > + > +using namespace std; > > Added: clang-tools-extra/trunk/test/change-namespace/white-list.cpp > URL: > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/change-namespace/white-list.cpp?rev=296110&view=auto > > ============================================================================== > --- clang-tools-extra/trunk/test/change-namespace/white-list.cpp (added) > +++ clang-tools-extra/trunk/test/change-namespace/white-list.cpp Fri Feb > 24 05:54:45 2017 > @@ -0,0 +1,19 @@ > +// RUN: echo "^std::.*$" > %T/white-list.txt > +// RUN: clang-change-namespace -old_namespace "na::nb" -new_namespace > "x::y" --file_pattern ".*" --whitelist_file %T/white-list.txt %s -- | sed > 's,// CHECK.*,,' | FileCheck %s > + > +#include "Inputs/fake-std.h" > + > +// CHECK: namespace x { > +// CHECK-NEXT: namespace y { > +namespace na { > +namespace nb { > +void f() { > + std::STD x1; > + STD x2; > +// CHECK: {{^}} std::STD x1;{{$}} > +// CHECK-NEXT: {{^}} STD x2;{{$}} > +} > +// CHECK: } // namespace y > +// CHECK-NEXT: } // namespace x > +} > +} > > Modified: > clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp > URL: > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp?rev=296110&r1=296109&r2=296110&view=diff > > ============================================================================== > --- > clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp > (original) > +++ > clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp > Fri Feb 24 05:54:45 2017 > @@ -38,7 +38,8 @@ public: > > std::map<std::string, tooling::Replacements> FileToReplacements; > change_namespace::ChangeNamespaceTool NamespaceTool( > - OldNamespace, NewNamespace, FilePattern, &FileToReplacements); > + OldNamespace, NewNamespace, FilePattern, > + /*WhiteListedSymbolPatterns*/ {}, &FileToReplacements); > ast_matchers::MatchFinder Finder; > NamespaceTool.registerMatchers(&Finder); > std::unique_ptr<tooling::FrontendActionFactory> Factory = > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
