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



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

Reply via email to