clang plugin: Compute absolute paths on Windows as well.

clang plugin: Compute absolute paths on Windows as well.

The motivation is to suppress diagnostics on the v8 bot,
which have "v8" somewhere on the path, but not in the
filename segments the compiler sees.  This makes the
behavior on Windows match the behavior on non-Windows.

BUG=644096
NOTRY=true

Committed: https://crrev.com/df0eb747fca2699d116ad80d0ccbd967317fdbd4
Committed: https://crrev.com/681568d4e7a08a988d677d8ad76d79faf87e7d2e
Cr-Original-Commit-Position: refs/heads/master@{#417701}
Cr-Commit-Position: refs/heads/master@{#417804}

[Nico, 2016-09-09 16:17:32]

Description was changed from

==========
clang plugin: Compute absolute paths on Windows as well.

BUG=644096
==========

to

==========
clang plugin: Compute absolute paths on Windows as well.

The motivation is to suppress diagnostics on the v8 bot,
which have "v8" somewhere on the path, but not in the
filename segments the compiler sees.  This makes the
behavior on Windows match the behavior on non-Windows.

BUG=644096
==========

[Nico, 2016-09-09 16:17:49]

thakis@chromium.org changed reviewers:
+ dcheng@chromium.org, hans@chromium.org

[Nico, 2016-09-09 16:18:40]

The CQ bit was checked by thakis@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-09 16:18:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-09 16:56:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-09 16:56:09]

Dry run: This issue passed the CQ dry run.

[dcheng, 2016-09-09 17:20:57]

https://codereview.chromium.org/2318733002/diff/40001/tools/clang/plugins/Chr...
File tools/clang/plugins/ChromeClassTester.cpp (right):

https://codereview.chromium.org/2318733002/diff/40001/tools/clang/plugins/Chr...
tools/clang/plugins/ChromeClassTester.cpp:143: // Use vector instead of string
because string doesn't guarantee continuous
According to my thorough scientific research of Google results, C++11 requires
contiguous storage for string.

[Nico, 2016-09-09 17:52:14]

https://codereview.chromium.org/2318733002/diff/40001/tools/clang/plugins/Chr...
File tools/clang/plugins/ChromeClassTester.cpp (right):

https://codereview.chromium.org/2318733002/diff/40001/tools/clang/plugins/Chr...
tools/clang/plugins/ChromeClassTester.cpp:143: // Use vector instead of string
because string doesn't guarantee continuous
On 2016/09/09 17:20:57, dcheng wrote:
> According to my thorough scientific research of Google results, C++11 requires
> contiguous storage for string.

Done. (Why didn't the make data() return non-const char*? :-/)

[Nico, 2016-09-09 17:52:21]

The CQ bit was checked by thakis@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-09 17:52:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-09 18:38:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-09 18:38:38]

Dry run: This issue passed the CQ dry run.

[dcheng, 2016-09-09 18:57:30]

https://codereview.chromium.org/2318733002/diff/60001/tools/clang/plugins/Chr...
File tools/clang/plugins/ChromeClassTester.cpp (right):

https://codereview.chromium.org/2318733002/diff/60001/tools/clang/plugins/Chr...
tools/clang/plugins/ChromeClassTester.cpp:145: std::wstring utf16(size_needed,
0);
Nit: '\0' maybe, for slightly better clarity?

https://codereview.chromium.org/2318733002/diff/60001/tools/clang/plugins/Chr...
tools/clang/plugins/ChromeClassTester.cpp:149: size_needed =
GetFullPathName(utf16.data(), 0, nullptr, nullptr) + 1; // nul
Nit: let's be consistent about calling the W versions.

https://codereview.chromium.org/2318733002/diff/60001/tools/clang/plugins/Chr...
tools/clang/plugins/ChromeClassTester.cpp:149: size_needed =
GetFullPathName(utf16.data(), 0, nullptr, nullptr) + 1; // nul
Also:

If the lpBuffer buffer is too small to contain the path, the return value is the
size, in TCHARs, of the buffer that is required to hold the path and the
terminating null character.

So I think this already includes the terminating null.

[Nico, 2016-09-09 19:06:33]

https://codereview.chromium.org/2318733002/diff/60001/tools/clang/plugins/Chr...
File tools/clang/plugins/ChromeClassTester.cpp (right):

https://codereview.chromium.org/2318733002/diff/60001/tools/clang/plugins/Chr...
tools/clang/plugins/ChromeClassTester.cpp:145: std::wstring utf16(size_needed,
0);
On 2016/09/09 18:57:29, dcheng wrote:
> Nit: '\0' maybe, for slightly better clarity?

meh, it's overwritten below anyways. And wouldn't it have to be L'\0'?

https://codereview.chromium.org/2318733002/diff/60001/tools/clang/plugins/Chr...
tools/clang/plugins/ChromeClassTester.cpp:149: size_needed =
GetFullPathName(utf16.data(), 0, nullptr, nullptr) + 1; // nul
On 2016/09/09 18:57:29, dcheng wrote:
> Nit: let's be consistent about calling the W versions.

Oh oops, done, thanks!

https://codereview.chromium.org/2318733002/diff/60001/tools/clang/plugins/Chr...
tools/clang/plugins/ChromeClassTester.cpp:149: size_needed =
GetFullPathName(utf16.data(), 0, nullptr, nullptr) + 1; // nul
On 2016/09/09 18:57:29, dcheng wrote:
> Also:
> 
> If the lpBuffer buffer is too small to contain the path, the return value is
the
> size, in TCHARs, of the buffer that is required to hold the path and the
> terminating null character.
> 
> So I think this already includes the terminating null.

You're right, I misread the docs. Done.

[Nico, 2016-09-09 19:06:56]

The CQ bit was checked by thakis@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-09 19:07:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dcheng, 2016-09-09 19:09:48]

lgtm

https://codereview.chromium.org/2318733002/diff/60001/tools/clang/plugins/Chr...
File tools/clang/plugins/ChromeClassTester.cpp (right):

https://codereview.chromium.org/2318733002/diff/60001/tools/clang/plugins/Chr...
tools/clang/plugins/ChromeClassTester.cpp:145: std::wstring utf16(size_needed,
0);
On 2016/09/09 19:06:33, Nico wrote:
> On 2016/09/09 18:57:29, dcheng wrote:
> > Nit: '\0' maybe, for slightly better clarity?
> 
> meh, it's overwritten below anyways. And wouldn't it have to be L'\0'?

I think '\0' should work fine here: the L prefix is only a problem for string
literals since the pointer types aren't convertible.

(It's just a visual hint to the reader that the first parameter is the count:
even though we're passing a named variable as the count, there's still a bare 0
integer literal right next to it, so it takes me a little extra mental energy to
remember that this is a character, not NULL or a count of some other sort)

[Nico, 2016-09-09 19:20:39]

The CQ bit was checked by thakis@chromium.org

[Nico, 2016-09-09 19:20:39]

The patchset sent to the CQ was uploaded after l-g-t-m from dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2318733002/#ps100001
(title: "less mental energy for dcheng")

[commit-bot: I haz the power, 2016-09-09 19:21:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Nico, 2016-09-09 19:25:28]

Thanks!

[dcheng, 2016-09-09 19:26:40]

On 2016/09/09 19:25:28, Nico wrote:
> Thanks!

Thanks, sorry I'm bad at remembering std::string's constructors =P

[Nico, 2016-09-09 20:25:04]

The CQ bit was unchecked by thakis@chromium.org

[Nico, 2016-09-09 20:25:10]

Description was changed from

==========
clang plugin: Compute absolute paths on Windows as well.

The motivation is to suppress diagnostics on the v8 bot,
which have "v8" somewhere on the path, but not in the
filename segments the compiler sees.  This makes the
behavior on Windows match the behavior on non-Windows.

BUG=644096
==========

to

==========
clang plugin: Compute absolute paths on Windows as well.

The motivation is to suppress diagnostics on the v8 bot,
which have "v8" somewhere on the path, but not in the
filename segments the compiler sees.  This makes the
behavior on Windows match the behavior on non-Windows.

BUG=644096
NOTRY=true
==========

[Nico, 2016-09-09 20:25:14]

The CQ bit was checked by thakis@chromium.org

[commit-bot: I haz the power, 2016-09-09 20:25:42]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-09 20:56:11]

Description was changed from

==========
clang plugin: Compute absolute paths on Windows as well.

The motivation is to suppress diagnostics on the v8 bot,
which have "v8" somewhere on the path, but not in the
filename segments the compiler sees.  This makes the
behavior on Windows match the behavior on non-Windows.

BUG=644096
NOTRY=true
==========

to

==========
clang plugin: Compute absolute paths on Windows as well.

The motivation is to suppress diagnostics on the v8 bot,
which have "v8" somewhere on the path, but not in the
filename segments the compiler sees.  This makes the
behavior on Windows match the behavior on non-Windows.

BUG=644096
NOTRY=true
==========

[commit-bot: I haz the power, 2016-09-09 20:56:12]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-09-09 20:58:45]

Description was changed from

==========
clang plugin: Compute absolute paths on Windows as well.

The motivation is to suppress diagnostics on the v8 bot,
which have "v8" somewhere on the path, but not in the
filename segments the compiler sees.  This makes the
behavior on Windows match the behavior on non-Windows.

BUG=644096
NOTRY=true
==========

to

==========
clang plugin: Compute absolute paths on Windows as well.

The motivation is to suppress diagnostics on the v8 bot,
which have "v8" somewhere on the path, but not in the
filename segments the compiler sees.  This makes the
behavior on Windows match the behavior on non-Windows.

BUG=644096
NOTRY=true

Committed: https://crrev.com/df0eb747fca2699d116ad80d0ccbd967317fdbd4
Cr-Commit-Position: refs/heads/master@{#417701}
==========

[commit-bot: I haz the power, 2016-09-09 20:58:51]

Patchset 6 (id:??) landed as
https://crrev.com/df0eb747fca2699d116ad80d0ccbd967317fdbd4
Cr-Commit-Position: refs/heads/master@{#417701}

[hans, 2016-09-09 23:27:13]

A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/2323273004/ by hans@chromium.org.

The reason for reverting is: The bots are sad. From
https://build.chromium.org/p/chromium.fyi/builders/ClangToTLinux%20%28dbg%29/...


/b/c/b/ClangToTLinux__dbg_/src/tools/clang/plugins/ChromeClassTester.cpp: In
member function ‘bool
ChromeClassTester::InBannedDirectory(clang::SourceLocation)’:
/b/c/b/ClangToTLinux__dbg_/src/tools/clang/plugins/ChromeClassTester.cpp:129:41:
error: ‘resolvedPath’ was not declared in this scope
   } else if (realpath(filename.c_str(), resolvedPath)) {
                                         ^
/b/c/b/ClangToTLinux__dbg_/src/tools/clang/plugins/ChromeClassTester.cpp:130:33:
error: redeclaration of ‘char resolvedPath [4096]’
     char resolvedPath[MAXPATHLEN];
                                 ^
/b/c/b/ClangToTLinux__dbg_/src/tools/clang/plugins/ChromeClassTester.cpp:129:41:
error: ‘<typeprefixerror>resolvedPath’ previously declared here
   } else if (realpath(filename.c_str(), resolvedPath)) {
                                         ^.

[Nico, 2016-09-10 02:24:13]

Description was changed from

==========
clang plugin: Compute absolute paths on Windows as well.

The motivation is to suppress diagnostics on the v8 bot,
which have "v8" somewhere on the path, but not in the
filename segments the compiler sees.  This makes the
behavior on Windows match the behavior on non-Windows.

BUG=644096
NOTRY=true

Committed: https://crrev.com/df0eb747fca2699d116ad80d0ccbd967317fdbd4
Cr-Commit-Position: refs/heads/master@{#417701}
==========

to

==========
clang plugin: Compute absolute paths on Windows as well.

The motivation is to suppress diagnostics on the v8 bot,
which have "v8" somewhere on the path, but not in the
filename segments the compiler sees.  This makes the
behavior on Windows match the behavior on non-Windows.

BUG=644096
NOTRY=true

Committed: https://crrev.com/df0eb747fca2699d116ad80d0ccbd967317fdbd4
Cr-Commit-Position: refs/heads/master@{#417701}
==========

[Nico, 2016-09-10 02:24:18]

The CQ bit was checked by thakis@chromium.org

[Nico, 2016-09-10 02:24:18]

The patchset sent to the CQ was uploaded after l-g-t-m from dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2318733002/#ps120001
(title: "relan")

[commit-bot: I haz the power, 2016-09-10 02:24:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-10 02:28:14]

Description was changed from

==========
clang plugin: Compute absolute paths on Windows as well.

The motivation is to suppress diagnostics on the v8 bot,
which have "v8" somewhere on the path, but not in the
filename segments the compiler sees.  This makes the
behavior on Windows match the behavior on non-Windows.

BUG=644096
NOTRY=true

Committed: https://crrev.com/df0eb747fca2699d116ad80d0ccbd967317fdbd4
Cr-Commit-Position: refs/heads/master@{#417701}
==========

to

==========
clang plugin: Compute absolute paths on Windows as well.

The motivation is to suppress diagnostics on the v8 bot,
which have "v8" somewhere on the path, but not in the
filename segments the compiler sees.  This makes the
behavior on Windows match the behavior on non-Windows.

BUG=644096
NOTRY=true

Committed: https://crrev.com/df0eb747fca2699d116ad80d0ccbd967317fdbd4
Cr-Commit-Position: refs/heads/master@{#417701}
==========

[commit-bot: I haz the power, 2016-09-10 02:28:15]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-09-10 02:29:47]

Description was changed from

==========
clang plugin: Compute absolute paths on Windows as well.

The motivation is to suppress diagnostics on the v8 bot,
which have "v8" somewhere on the path, but not in the
filename segments the compiler sees.  This makes the
behavior on Windows match the behavior on non-Windows.

BUG=644096
NOTRY=true

Committed: https://crrev.com/df0eb747fca2699d116ad80d0ccbd967317fdbd4
Cr-Commit-Position: refs/heads/master@{#417701}
==========

to

==========
clang plugin: Compute absolute paths on Windows as well.

The motivation is to suppress diagnostics on the v8 bot,
which have "v8" somewhere on the path, but not in the
filename segments the compiler sees.  This makes the
behavior on Windows match the behavior on non-Windows.

BUG=644096
NOTRY=true

Committed: https://crrev.com/df0eb747fca2699d116ad80d0ccbd967317fdbd4
Committed: https://crrev.com/681568d4e7a08a988d677d8ad76d79faf87e7d2e
Cr-Original-Commit-Position: refs/heads/master@{#417701}
Cr-Commit-Position: refs/heads/master@{#417804}
==========

[commit-bot: I haz the power, 2016-09-10 02:29:48]

Patchset 7 (id:??) landed as
https://crrev.com/681568d4e7a08a988d677d8ad76d79faf87e7d2e
Cr-Commit-Position: refs/heads/master@{#417804}