RFR: 8307160: Fix AWT/2D/A11Y to support the permissive- flag on the Microsoft Visual C compiler [v18]

Phil Race prr at openjdk.org
Fri Nov 3 17:14:20 UTC 2023


On Fri, 3 Nov 2023 04:12:46 GMT, Julian Waters <jwaters at openjdk.org> wrote:

>> We should set the -permissive- flag for the Microsoft Visual C compiler, as was requested by the now backed out [JDK-8241499](https://bugs.openjdk.org/browse/JDK-8241499). Doing so makes the Visual C compiler much less accepting of ill formed code, which will improve code quality on Windows in the future.
>
> Julian Waters has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Part II awt_PrintJob.cpp

> > > Actually, I can do you one better, now that I think of it, so I don't have to explains potentially deeply nested and confusing error logs:
> > > ```
> > > int main() {
> > >     goto skip;
> > >     int i = 0;
> > >     skip:
> > >     std::printf("Done\n");
> > > }
> > > ```
> > > 
> > > 
> > >     
> > >       
> > >     
> > > 
> > >       
> > >     
> > > 
> > >     
> > >   
> > > C:\Users\vertig0\Downloads>cl.exe -nologo -std:c++14 -permissive- -Fo:goto.o goto.cpp
> > > goto.cpp(5): error C2362: initialization of 'c' is skipped by 'goto skip' goto.cpp(6): note: see declaration of 'skip'
> > > You simply cannot jump over initialization of a local with a goto in C++, as shown by the Visual C compiler above, does this answer your question?
> > 
> > 
> > It tells me 2 things (1) C++ is awful, because the change you made to silence it is a no-op (2) the changes you are making are really wrong, because C++ is telling us it does not like goto, and you aren't addresssing that
> 
> I made the decision to use no-op changes based on the review in #15996 at @djelinski's suggestion, because I assumed the best way forward is to make the changes minimally disruptive to client code, is that not what we're aiming for here? The quote-on-quote correct ways to fix this are either to wrap everything in a scope, or rewrite a significant portion of the AWT codebase to remove the gotos, the former is very messy and hard to review (as seen above in one of your earlier reviews), the latter would be difficult to do. Not sure which one you'd prefer?
> 
> Side note: What about the jdk.accessibility changes? Are those ok at least?

So this is "illegal" 

 int main() {
  goto skip;
int i = 0;
 skip:
std::printf("Done\n");
 }

and this is legal ?

 int main() {
  goto skip;
 int i;
 i= 0;
 skip:
std::printf("Done\n");
 }

Honestly, I don't like these changes at all.
Anyone looking at the code after the proposed change would just be itching to undo it unless they knew exactly why it was like that.
I'd prefer to leave the code alone and complain to Microsoft as I can't believe they've interpreted / implemented the C++ spec correctly. 
2nd choice is to re-write without gotos.

PS, yes the A11Y changes are OK.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/15096#issuecomment-1792828679
PR Comment: https://git.openjdk.org/jdk/pull/15096#issuecomment-1792829994


More information about the client-libs-dev mailing list