aaron.ballman added a comment.

In D148276#4324314 <https://reviews.llvm.org/D148276#4324314>, @sousajo wrote:

> have been sick, and could not advance much except I added the tests to 
> replicate the issue. Any ideas on how to proceed here?

Sorry to hear you've been sick, but thank you for your patience while I thought 
about this further. Two points worth observing:

1. GCC diagnoses the problematic code the same way your patch does: 
https://godbolt.org/z/44GnM9jzE
2. Clang and GCC both diagnose the notionally equivalent code using a C-style 
cast: https://godbolt.org/z/bbTPYb5rE

Based on that, it seems defensible for us to diagnose that code by re-landing 
the patch. However, I wonder why the changes broke anything. We build with GCC 
all the time. Is `-Wcast-qual` only enabled for bots building with Clang?

However, I also feel like this behavior is somewhat user-hostile because it 
seems reasonable to expect the developer is well aware that they're casting 
away the `const` qualifier when using something named `remove_const_t` to 
specify the cast destination type, so it seems potentially reasonable to 
silence the diagnostic in those cases (consistently, for both function- and 
c-style casts). It seems to be a reasonably popular approach to casting away 
const in the wild: 
https://sourcegraph.com/search?q=context:global+%5Cb%5Bstd::%5D%3Fremove_const%5B_t%5D%3F%5C%3C.*%5C%3E%5B::type%5D%3F%5C%28.*%5C%29&patternType=regexp&sm=1&groupBy=group
 so this might allow more folks to opt into `-Wcast-qual`. But implementing 
that would be tricky because we'd have to look through the cast expression to 
see whether it came from a type trait, and we typically do not want the 
frontend to be inspecting AST nodes by name (like looking for particular 
identifiers and changing behavior based on that), and we'd be introducing 
another divergence between the Clang and GCC behaviors.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148276/new/

https://reviews.llvm.org/D148276

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

Reply via email to