[PATCH] D52773: clang-cl: Add /showFilenames option (PR31957)

[Stephen Kelly via Phabricator via cfe-commits]

steveire added inline comments.


================
Comment at: lib/Frontend/CompilerInvocation.cpp:792
   Opts.MainFileName = Args.getLastArgValue(OPT_main_file_name);
+  if (Args.hasArg(OPT_echo_main_file_name))
+    llvm::outs() << Opts.MainFileName << "\n";
----------------
zturner wrote:
> zturner wrote:
> > steveire wrote:
> > > hans wrote:
> > > > zturner wrote:
> > > > > steveire wrote:
> > > > > > I'll have to keep a patch around anyway for myself locally to remove this condition. But maybe someone else will benefit from this.
> > > > > > 
> > > > > > What do you think about changing CMake to so that this is passed by default when using Clang-CL?
> > > > > > 
> > > > > Do you mean llvms cmake?  Or cmake itself?  Which generator are you using?
> > > > Can't you just configure your build to pass the flag to clang-cl?
> > > > 
> > > > I suppose CMake could be made to do that by default when using clang-cl versions that support the flag and using the MSBuild generator, but that's for the CMake community to decide I think. Or perhaps our VS integration could do that, but it gets tricky because it needs to know whether the flag is supported or not.
> > > I mean upstream cmake. My suggestion is independent of the generator, but it would depend on what Brad decides anyway. I don't intend to implement it, just report it.
> > > 
> > > I only have one clang but I have multiple buildsystems, and I don't want to have to add a new flag too all of them. In each toplevel CMakeLists everyone will need this to make use of the flag:
> > > 
> > > ```
> > > 
> > > # Have to remember to use "${CMAKE_CXX_SIMULATE_ID}" instead of 
> > > # Undecorated "CMAKE_CXX_SIMULATE_ID" because there is a 
> > > # variable named "MSVC" and
> > > # the way CMake variables and the "if()" command work requires this. 
> > > if (CMAKE_CXX_COMPILER_ID STREQUAL Clang 
> > >         AND ${CMAKE_CXX_SIMULATE_ID} STREQUAL MSVC)
> > >     # CMake does not have explicit Clang-CL support yet.
> > >     # It should have a COMPILER_ID for it.
> > >     set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /showFilename")
> > >     # etc
> > > endif()
> > > ```
> > > 
> > > Upstream CMake can have the logic to add the flag instead.
> > But then what if you don’t want it?  There would be no way to turn it off.  I don’t think it’s a good fit for cmake 
> Yes, and actually for this reason i was wondering if we can do it without a flag at all.  What if we just set an magic environment variable?  It handles Stephen’s use case (he can just set it globally), and MSBuild (we can set it in the extension).
> But then what if you don’t want it? There would be no way to turn it off. 

Oh, I thought that the last flag would dominate. ie, if cmake generated `/showFilename` and the user adds `/showFilename-`, then you would end up with  

`clang-cl.exe /showFilename /showFilename-` 

and it would not be shown. I guess that's not how it works.

Maybe users want to not show it, but not seeing the filename is a surprising first-time experience when porting from MSVC to clang-cl using Visual Studio.

However, I'll just drop out of the discussion and not make any bug to CMake. If anyone else feels strongly, they can do so.

Thanks!


https://reviews.llvm.org/D52773