curdeius added a subscriber: curdeius.
curdeius added a comment.
Some minor remarks. Sorry for being finicky :).
================
Comment at: include/clang/Basic/VirtualFileSystem.h:97
@@ +96,3 @@
+ return Status->getName();
+ else
+ return Status.getError();
----------------
No else needed after return.
================
Comment at: lib/Lex/PPDirectives.cpp:1673
@@ +1672,3 @@
+ if (File && !File->tryGetRealPathName().empty() &&
+ !Diags->isIgnored(diag::pp_nonportable_path, FilenameTok.getLocation())) {
+ StringRef Name = LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename;
----------------
May you create a bool variable for this long condition and name it so that it's
clear what it checks?
================
Comment at: lib/Lex/PPDirectives.cpp:1678-1700
@@ +1677,25 @@
+ llvm::sys::path::end(Name));
+ auto iRealPathComponent = llvm::sys::path::rbegin(RealPathName);
+ auto iRealPathComponentEnd = llvm::sys::path::rend(RealPathName);
+ int Cnt = 0;
+ bool SuggestReplacement = false;
+ for(auto& Component : llvm::reverse(Components)) {
+ if ("." == Component) {
+ } else if (".." == Component) {
+ ++Cnt;
+ } else if (Cnt) {
+ --Cnt;
+ } else if (iRealPathComponent != iRealPathComponentEnd) {
+ if (Component != *iRealPathComponent) {
+ // If these path components differ by more than just case, then we
+ // may be looking at symlinked paths. Bail on this diagnostic to
avoid
+ // noisy false positives.
+ SuggestReplacement = iRealPathComponent->equals_lower(Component);
+ if (!SuggestReplacement)
+ break;
+ Component = *iRealPathComponent;
+ }
+ ++iRealPathComponent;
+ }
+ }
+ if (SuggestReplacement) {
----------------
Could you extract a function for this chunk? Something like `bool
SuggestReplacement = simplifyPath(Components);`.
The `HandleIncludeDirective` method is already loooong enough.
http://reviews.llvm.org/D19843
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
