User Details
- User Since
- Oct 9 2013, 1:30 PM (391 w, 6 d)
Oct 7 2019
Feb 28 2018
Jun 12 2017
Added test in r305213.
Jun 11 2017
Apr 27 2017
OK
Raphael , do you have commit access? should I commit this?
Apr 25 2017
LGTM
Please move the =new out of the PP.AddPragmaHandler calls.
While at it, this code still (as the original) leaks the PragmaHandlers. These should be deleted after RemovePragmaHandler or instead, simply use std::unique_ptr.
Apr 17 2017
reverted in r300497
Apr 14 2017
Adding Mateusz and Reid.
Apr 12 2017
Added attribute test.
Apr 5 2017
Thanks, I'll make a test.
Mar 31 2017
Mar 30 2017
Mar 29 2017
No intention of any major work intended... just moved the code (locally) to Release and regression tests were still passing.
Most other module flags are added at CodeGenModule::Release(). For consistency, could this code be in CodeGenModule::Release() as well?
Mar 21 2017
Could use dumpDeclContext() to test?
Feb 14 2017
OK.
If something is broken now we don't break it even more.
__float128 remain be fixed to be compatible, not only some poor developer would have to fix the missing headers bug one day, he will have to re-fix limits.h the right way and undo this "fix".
There is a problems with limits.h, fix limits.h. Don't make all headers that happens to be in the same directory as limits.h disappear.
Feb 13 2017
These directories are "mostly" equivalent, with some headers existing is mings gcc dir but missing in clang resource dir and thus will break compilation.
For example, the most popular (37089 download this week) https://sourceforge.net/projects/mingw-w64 distribuion for Windows had elected to place the includes
omp.h, quadmath.h, openacc.h
at
c:\mingw32\lib\gcc\i686-w64-mingw32\5.4.0\include\
with the corresponding libraries at
c:\mingw32\lib\gcc\i686-w64-mingw32\5.4.0\
I had verified this still holds true for the latest i686-6.3.0-release-posix-dwarf-rt_v5-rev1.7z package.
Removing the include dir will make clang less useful for mingw users using these includes, since they will not be found anymore.
Furthermore, It makes no sense to make such intrusive wide-ranging change where the original problem was with limits.h is actually due to:
/* The system's limits.h may, in turn, try to #include_next GCC's limits.h. Avert this #include_next madness. */ #if defined __GNUC__ && !defined _GCC_LIMITS_H_ #define _GCC_LIMITS_H_ #endif
so the solution may be as simple as
#if defined __GNUC__ && !defined _GCC_LIMITS_H_ && !defined __MINGW32__ #define _GCC_LIMITS_H_ #endif
Feb 9 2017
What about omp.h and openacc.h ? many programs are using OpenMP.
The gcc include dirs in mingw contains headers not available esewhere and thus can't be removed.
Notable examples,
Feb 3 2017
Hiding these two include dirs removes many headers. Most has clang equivalents but not all of them.
For example quadmath.h is only there, and without the include path programs using it will fail to compile.
This code is actually used with Windows as well as Linux (with the exception of line 218), see the comment blocks above for detailed include dirs from all platforms from which it was derived.
Jan 21 2017
LGTM, matches the code in libstdc++ stdarg,h.
You can remove the 'hack' comment in line 46, __GNUC_VA_LIST is just a standard include guard for the typedef.
Jan 12 2017
Jan 6 2017
While at it, http://llvm.org/pr30994
Dec 29 2016
Dec 12 2016
Nov 29 2016
LGTM after fixing the inline comment
Nov 26 2016
Jul 28 2016
PR28749 was created with trunk clang on Ubuntu x64.
It reproduced reliably but is very fragile, changing just about anything in the code makes the problem disappear.
Jul 27 2016
I added a small reproducer for what is probably the same problem in
Jun 8 2016
You mean
Some target platforms do not support -fsanitize=address.
May 13 2016
result of r269402, LGTM
Apr 20 2016
Ah, OK, I am using regular svn and thus did not counter this problem.
Could you detail this exactly, the problem happens when using git-svn with core.autocrlf config=true but not with svn?
LGTM with this addition.
AFAIK the eol type is decided by svn based the svn:eol-style property, so as long the files were checked in correctly and you do not override it specifically while svn checkout, it just works.
Did you encounter problems with eol ?
Feb 17 2016
Would be nice to have CodeGenAction::TheModule redirect to CodeGeneratorImpl::M if possible, but that's for another patch. LGTM.
It certainly makes sense to redirect the module request to its owner instead of duplicating it in a local copy.
Feb 11 2016
We have tried to keep one copy of DataLayout around
http://reviews.llvm.org/D11103
Can it share the Module->getDataLayout() ?
Jan 28 2016
The instantiated does get a new collection of ParmVarDecls while getting
the pattern Type which points to the pattern ParmVarDecls. So the
ParmVarDecls pointed in the instantiated are not the same as those pointed
by its Type.
Dec 18 2015
#define NOGDI
is also useful.
+1 all general-purpose Windows stuff should move to WindowsSupport.h. I'm surprised we got away without so far.
We have NOMINMAX at lib/Driver/MSVCToolChain.cpp before Windows.h which should probably switched to use WindowsSupport.h.
We don't support building with mingw.org.
mingw-w64 has its own versionhelpers.h with IsWindows8OrGreater(), at least from gcc 4.93, probably earlier.
Dec 16 2015
LGTM with some tests cases.
Dec 14 2015
MSVC 2013 Update 5 accepts for (bool SkipUnwritten : {false, true}).
Possibly changed in one of the Updates?
Dec 3 2015
Only abstracting the whole llc/opt drivers which is more trouble than this is worth.
Maybe a commment in llc that opt does something similar and vice versa.
LGTM.
Dec 2 2015
Do you see any reasonable way to share the duplicated code between llc.cpp and opt.cpp?
Dec 1 2015
There is another copy of getEffectiveDeclContext in MicrosoftMangle.cpp which pre-commit was identical to this one.
Should it get this commit as well to keep both identical?
If so, could both copies of getEffectiveDeclContext be merged?
Nov 26 2015
findGccDir() can return llvm::ErrorOr<std::string> and then all Base assignments happen at the same if-elseif-else:
Nov 25 2015
This always searches for something-gcc and then discards the result if sysroot was provided, which is a waste.
Move the searching to a helper function and then it can be done only if sysroot was not provided, as it is now.
You are right, running passes over again for all but trivial code will not result in the same module.
The CloneModule approach should work.
Won't the second run output will overwrite the first run output?
If so, what do you think about renaming the second output so it may be binary compared to the first one?
Nov 24 2015
This makes sense.
We have been fixed such bugs before, usually when a data member is added to one of the Streamers, so LGTM.
That's a great idea. Maybe add a LIT test that uses this flag so breakages will be caught at the buildbots?
Nov 20 2015
Nov 19 2015
Visual C++ does not support hexadecimal floating point literals.
Since all of these are actually integers they should work without the ".0" and ".0f".
Nov 11 2015
Hi Andy,
Nov 4 2015
The formatting is wrong, aligned to the left, clang-format the new code.
Neat and efficient, LGTM
Nov 3 2015
We did not have cfe-commits as subscriber so I'm adding it now (this is a clang commit), see if someone would like to further comment.
I see the usage pattern is CopyEnvironment, push_back something, use std::vector<const char *> with ExecuteAndWait. For this you must have a consecutive vector of char * in memory, so someone will have to manage their lifetime. It's better not to leave this to the caller or split responsibility so you could objectize CopyEnvironment to do everything required:
Oct 29 2015
This code was my first thought too, but I had been commented on the structure of very similar code in this file:
Yes, I think this is a better default than /usr.
The code could now be merged (untested). I disconnected the third part on purpose based on comment from Joerg Sonnenberger on the previous code that having else after #endif is confusing.
Oct 1 2015
clang can't generate Visual C++ debug info except line numbers yet, so building with VC is essential to development on Windows. While most people and our bots are still using VC 2013 going forward supporting VC 2015 (even with bugs) is worth a small patch.
Sep 27 2015
Please see http://llvm.org/pr24952
Sep 26 2015
Hi Dan, it makes sense that output streams should not usually be mixed together, especially if one is binary as you write.
This may or may not be a problem depending on what the user really wants. He may want to mix the outputs for whatever purposes or it may usually be a user error.
In any case, that's not how clang deal with usage or even internal unexpected errors. It asserts, print error messages but does not crash on purpose. Having clang crash here does not seem like a good solution and will result in this issue continue to surface again.
Sep 24 2015
The original commit http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20100816/106268.html by Dan Ghoman says:
When stdout goes elsewhere the console, the shell creates the the output file (pipe) and will close it when clang terminates so so why clang should close it at all ? it did not open it.
Sep 17 2015
Sep 8 2015
The asserts on unit tests, BitVectorTest/1.PortableBitMask that calls
Sep 4 2015
Sep 3 2015
In CGCXX.cpp, may be fixable after this commit:
Aug 29 2015
I have never used OSX, Try to add one of the Apple clang developers as reviewers, they know more than me about OSX.
Aug 15 2015
Fixed several ThreadSanitizer.cpp issues in r245167. I don't have VS2015 config to test the warning and the online compiler is out but
Committed the X86ISelLowering.cpp patch in r245161.
Aug 13 2015
I think she wishes your second option.
Aug 12 2015
Yes, go ahead.
Aug 11 2015
Which tests? Can this be solved with a script for %run? A substitution in lit? It seems like a bug somewhere else. Why is TZ not being passed while other env vars are?
The bash shell on Windows from MSYS expands anyhting that looks like a relative path, such as /m. It has workaround for switches but not for environment vas so tests using 'env' were failing without any way to fix. In addition, it did not pass environment variable TZ since it does not know how ot map between Unix and Windows TZ. This also broke tests.
There is also the case insensitivity issue, see
Aug 10 2015
In what configuration: VC, target do you see the warnings?