Assigned
Status Update
No update yet.
Comments
as...@chromium.org
+1
I had the same issue onhttps://crrev.com/c/3350351
See screenshothttp://screen/3wdchJYHkCavyS9
I tried clicking "Apply Fix" just in case my eyes were deceiving me and there actually was a diff to apply, but Gerrit predictably returned an error because there are no changes to be applied
I had the same issue on
See screenshot
I tried clicking "Apply Fix" just in case my eyes were deceiving me and there actually was a diff to apply, but Gerrit predictably returned an error because there are no changes to be applied
dd...@google.com
[Empty comment from Monorail migration]
gb...@chromium.org
Thanks for this! Seems like it could be a bug in clang-tidy; will dig soon :)
gb...@chromium.org
[Empty comment from Monorail migration]
gb...@chromium.org
Yeah, looking at the code, this is likely a clang-tidy bug. As of 2fd7755c2d2818bfac7a367c96188c22cd99b413 (HEAD~20000), this warning wasn't firing; will bisect to verify that a clang-tidy update introduced this && go from there
gb...@chromium.org
[Empty comment from Monorail migration]
gb...@chromium.org
Oooh, this wasn't a clang-tidy update like I expected. a535562fad3f33f419c3e245953706f5a1a309fc is the culprit, which is a more general flag flip to c++17 for linux.
It's intersting that clang-tidy only behaves this way in c++17 mode. I'll try to reduce this test-case and repro with that directly.
It's intersting that clang-tidy only behaves this way in c++17 mode. I'll try to reduce this test-case and repro with that directly.
gb...@chromium.org
`creduce` gave me:
```
typedef float a;
struct b {};
class c {
public:
c() : c(1, 0, 0, 0, 1, 0, 0, 0, 1, d) {}
bool e(b *);
int d;
c(a, a, a, a, a, a, a, a, a, int);
};
namespace {
class f {
public:
operator c() const;
};
class g;
class h {
void i(g *) const;
f j;
};
void h::i(g *) const {
b k;
c(j).e(&k);
}
} // namespace
```
and reducing flags gave me `clang-tidy '-checks=-*,google-readability-casting' repro.cpp -- -std=c++17`.
Seems small enough to either glance at myself or file upstream. If I can find time before the 7th, I'll see what I can do. Otherwise, I'll file an upstream bug. :)
```
typedef float a;
struct b {};
class c {
public:
c() : c(1, 0, 0, 0, 1, 0, 0, 0, 1, d) {}
bool e(b *);
int d;
c(a, a, a, a, a, a, a, a, a, int);
};
namespace {
class f {
public:
operator c() const;
};
class g;
class h {
void i(g *) const;
f j;
};
void h::i(g *) const {
b k;
c(j).e(&k);
}
} // namespace
```
and reducing flags gave me `clang-tidy '-checks=-*,google-readability-casting' repro.cpp -- -std=c++17`.
Seems small enough to either glance at myself or file upstream. If I can find time before the 7th, I'll see what I can do. Otherwise, I'll file an upstream bug. :)
gb...@chromium.org
[Empty comment from Monorail migration]
gb...@chromium.org
After chatting offline, sounds like ryanbeltran@ is a more appropriate owner.
For a bit more context, the warning we're getting that we shouldn't be on ToT is:
```
clang-tidy -checks='-*,google-readability-casting' repro.cpp -- -std=c++17
3 warnings generated.
/usr/local/google/home/gbiv/crbug-creduce/repro.cpp:22:3: warning: C-style casts are discouraged; use constructor call syntax [google-readability-casting]
c(j).e(&k);
^
c
Suppressed 2 warnings (2 with check filters).
```
`c(j)` isn't a C-style cast, and the fixit ends up being a nop.
For a bit more context, the warning we're getting that we shouldn't be on ToT is:
```
clang-tidy -checks='-*,google-readability-casting' repro.cpp -- -std=c++17
3 warnings generated.
/usr/local/google/home/gbiv/crbug-creduce/repro.cpp:22:3: warning: C-style casts are discouraged; use constructor call syntax [google-readability-casting]
c(j).e(&k);
^
c
Suppressed 2 warnings (2 with check filters).
```
`c(j)` isn't a C-style cast, and the fixit ends up being a nop.
gb...@chromium.org
[Empty comment from Monorail migration]
ef...@chromium.org
[Empty comment from Monorail migration]
[Monorail components: Infra>LUCI>BuildService>PreSubmit>Tricium]
[Monorail components: Infra>LUCI>BuildService>PreSubmit>Tricium]
ef...@chromium.org
[Empty comment from Monorail migration]
[Monorail components: -Infra>Platform>Tricium>Analyzer]
[Monorail components: -Infra>Platform>Tricium>Analyzer]
gb...@google.com
[Empty comment from Monorail migration]
gb...@chromium.org
[Empty comment from Monorail migration]
gb...@chromium.org
Friendly ping -- any progress on this? Looks like folks are bumping into it occasionally
wa...@chromium.org
I'm bumping this frequently (almost every day) and I'm ignoring the warnings.
th...@chromium.org
Should we disable this diag for now?
gi...@appspot.gserviceaccount.com
The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src/+/f828a2248f0da4e7bfadfa5db324230043aaea02
commit f828a2248f0da4e7bfadfa5db324230043aaea02
Author: George Burgess IV <gbiv@google.com>
Date: Tue Apr 19 19:30:15 2022
clang-tidy: temporarily disable google-readability-casting
The issue pointed out incrbug.com/1282228 seems quite noisy for some
folks. Disabling this lint until it's fixed seems like the best way
forward.
Bug: 1282228
Change-Id: I7bc2fd2eb152e644183fdb929716a8e2d5eba223
Reviewed-on:https://chromium-review.googlesource.com/c/chromium/src/+/3594061
Commit-Queue: George Burgess <gbiv@chromium.org>
Auto-Submit: George Burgess <gbiv@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/main@{#993839}
[modify]https://crrev.com/f828a2248f0da4e7bfadfa5db324230043aaea02/.clang-tidy
commit f828a2248f0da4e7bfadfa5db324230043aaea02
Author: George Burgess IV <gbiv@google.com>
Date: Tue Apr 19 19:30:15 2022
clang-tidy: temporarily disable google-readability-casting
The issue pointed out in
folks. Disabling this lint until it's fixed seems like the best way
forward.
Bug: 1282228
Change-Id: I7bc2fd2eb152e644183fdb929716a8e2d5eba223
Reviewed-on:
Commit-Queue: George Burgess <gbiv@chromium.org>
Auto-Submit: George Burgess <gbiv@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/main@{#993839}
[modify]
is...@google.com
bl...@google.com
Automated by Blunderbuss job peepsi_source_blunderbuss_autoassigner for component 1456212.
ri...@chromium.org
In c(j) is actually a cast, as if you had said int(j). AFAIK, this isn't valid syntax in C so I wouldn't call it a c-style cast personally. The style guide says "You may use cast formats like T(x) only when T is a class type." but here c is a class type so it's not a valid warning by the style guide.
It no longer gives this warning for this repro, so maybe there is hope that it is fixed?
fl...@google.com
dx...@google.com
Project: chromium/src
Branch: main
Author: Julia Hansbrough
Link:
[clang-tidy] Reenable google-readability-casting.
Expand for full commit details
The "no-op suggested fixes" issue has been fixed in https://github.com/llvm/llvm-project/pull/71650 and no longer raises false positives on the code referenced in https://issues.chromium.org/issues/40209512 , so it should be sound to re-enable this clang-tidy warning.
Bug: 40209512
Change-Id: I8b9464f4c5b8b48705d5dd46ba6ad762b847c442
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6341202
Commit-Queue: Julia Hansbrough <flowerhack@google.com>
Reviewed-by: Rick Byers <rbyers@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1432258}
Files:
- M
.clang-tidy
Hash: 19cf10885857a8b20a83a7838dcd9f69910a3860
Date: Thu Mar 13 18:31:09 2025
Description
In
{"file_path": "ui/gfx/geometry/transform.cc", "line_number": 504, "diag_name": "google-readability-casting", "message": "C-style casts are discouraged; use constructor call syntax", "replacements": [{"new_text": "SkRRect", "start_line": 504, "end_line": 504, "start_char": 7, "end_char": 14}], "expansion_locs": [], "notes": []}
where it's suggesting replacing "SKRRect" with the exact same text.
It's not clear to me whether it was trying to suggest something different or whether it instead shouldn't have suggested a change.