baloghadamsoftware created this revision.
baloghadamsoftware added a reviewer: NoQ.
baloghadamsoftware added a project: clang.
Herald added subscribers: donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs,
szepet.
Herald added a reviewer: george.karpenkov.
Some operations, especially iterator compar
baloghadamsoftware added a comment.
I marked this patch as WIP because I could not create a test-case for it.
However in real projects this patch seems to reduce false positives
significantly.
The member function `getBaseRegion()` of `MemRegion` does not work here because
it recursively retrie
Author: hans
Date: Tue Nov 13 01:05:12 2018
New Revision: 346748
URL: http://llvm.org/viewvc/llvm-project?rev=346748&view=rev
Log:
UserManual: Tweak the /Zc:dllexportInlines- docs some
Addressing comments on https://reviews.llvm.org/D54319
Modified:
cfe/trunk/docs/UsersManual.rst
cfe/tru
hans marked 3 inline comments as done.
hans added a comment.
Thanks for the review! I made adjustments in r346748.
Comment at: cfe/trunk/docs/UsersManual.rst:3104
+
+This causes the class-level `dllexport` and `dllimport` attributes not to be
+applied to inline member functions
klimek added a comment.
Thanks Aaron for volunteering, I'm very thankful for all your work on the
reviews, it's much appreciated :D
Repository:
rL LLVM
https://reviews.llvm.org/D54453
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http:
steveire updated this revision to Diff 173819.
steveire added a comment.
Update
Repository:
rC Clang
https://reviews.llvm.org/D54405
Files:
include/clang/ASTMatchers/ASTMatchersInternal.h
lib/ASTMatchers/Dynamic/Registry.cpp
Index: lib/ASTMatchers/Dynamic/Registry.cpp
=
steveire updated this revision to Diff 173820.
steveire added a comment.
Update
Repository:
rC Clang
https://reviews.llvm.org/D54402
Files:
lib/ASTMatchers/Dynamic/Registry.cpp
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
steveire added a comment.
Let's discuss it on IRC at some point and see if we can come up with wording.
Repository:
rC Clang
https://reviews.llvm.org/D54404
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mai
steveire added inline comments.
Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:77
+ internal::MatcherDescriptor *matchDescriptor, StringRef MatcherName) {
+auto K = ast_type_traits::ASTNodeKind::getFromNodeKind<
+typename
ast_matchers::internal::VariadicAllOfM
Szelethus added a comment.
Herald added a subscriber: baloghadamsoftware.
Allow me to disagree again -- the reason why `CheckerRegistry` and the entire
checker registration process is a mess is that suboptimal solutions were added
instead of making the system do well on the long term. This is co
richardmembarth added a comment.
CUDA maps `__shared__` internally also to `__attribute__((shared))`:
#define __annotate__(a) \
__attribute__((a))
#define __location__(a) \
__annotate__(a)
...
#define __shared__ \
__location__(shared)
My guess is that Clang
kadircet updated this revision to Diff 173824.
kadircet marked 18 inline comments as done.
kadircet added a comment.
- Address comments.
- Move loading from cache out of indexing.
- Use a factory for creation of index storage.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54269
kadircet added inline comments.
Comment at: clangd/index/Background.h:39
+ retrieveShard(llvm::StringRef ShardIdentifier, FileDigest Hash) const = 0;
+ virtual bool initialize(llvm::StringRef Directory) = 0;
+};
kadircet wrote:
> sammccall wrote:
> > kadircet w
arichardson updated this revision to Diff 173828.
arichardson added a comment.
rebased on latest HEAD
Repository:
rC Clang
https://reviews.llvm.org/D40016
Files:
lib/Sema/SemaOverload.cpp
Index: lib/Sema/SemaOverload.cpp
===
sammccall added a comment.
Forgot to say - the scope here looks just right, thanks for slimming this down!
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52273
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llv
sammccall added inline comments.
Comment at: clangd/ExpectedTypes.cpp:12
+
+static llvm::Optional toEquivClass(ASTContext &Ctx, QualType T) {
+ if (T.isNull() || T->isDependentType())
returning QualType vs Type*? It seems we strip all qualifiers, seems clearest
alexfh added a comment.
> Thanks Aaron for volunteering, I'm very thankful for all your work on the
> reviews, it's much appreciated :D
+1 ;)
Repository:
rL LLVM
https://reviews.llvm.org/D54453
___
cfe-commits mailing list
cfe-commits@lists.llv
wuzish added inline comments.
Comment at: clang/test/CodeGen/altivec-generic-overload.c:74
+ convert1(gv1);
+ // CHECK: call void @_Z8convert1Dv16_a(<16 x i8> %{{[0-9]+}})
+ convert1(gv2);
hubert.reinterpretcast wrote:
> Checking that the call is to the expect
sdesmalen marked an inline comment as done.
sdesmalen added inline comments.
Comment at: include/clang/Basic/Attr.td:1792
+ let Spellings = [GNU<"aarch64_vector_pcs">,
+ CXX11<"clang", "aarch64_vector_pcs">,
+ Keyword<"__aarch64_vector_pcs">,
sdesmalen updated this revision to Diff 173831.
sdesmalen added a comment.
- Removed `_aarch64_vector_pcs` and `__aarch64_vector_pcs` keywords in favour
of supporting only `__attribute__(aarch64_vector_pcs))`.
https://reviews.llvm.org/D54425
Files:
include/clang-c/Index.h
include/clang/Bas
ChristophBerg added a comment.
PostgreSQL 11 is now using LLVM to do JITing of SQL expressions. We'd need this
feature to strip the build directory off the .bc bitcode files so the .deb
packages build reproducibly.
@dankm: Are you still working on this? What can we do to help getting this move
grimar added inline comments.
Comment at: lib/Driver/ToolChains/CommonArgs.cpp:813-830
+ if (Arg *A = Args.getLastArg(options::OPT_gsplit_dwarf_EQ))
+if (StringRef(A->getValue()) == "single")
+ return Args.MakeArgString(Output.getFilename());
+
Arg *FinalOutput = Arg
gchatelet updated this revision to Diff 173836.
gchatelet marked 6 inline comments as done.
gchatelet added a comment.
- Address comments
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53488
Files:
clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
clang-tidy/cppcor
gchatelet added inline comments.
Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:299
+ Rhs->isCXX11ConstantExpr(Context, &Constant)) {
+if (Constant.isFloat())
+ diagIfNarrowFloatingPointConstant(Context, SourceLoc, Lhs, Rhs,
Constant);
-
Szelethus planned changes to this revision.
Szelethus added a comment.
Here's how I'm planning to approach this issue:
- Split `MallocChecker` and `CStringChecker` up properly, introduce
`MallocCheckerBase` and `CStringCheckerBase`. I'm a little unsure about how
I'll do this, but I'll share my
sammccall added inline comments.
Comment at: clangd/index/Background.cpp:52
+llvm::SmallString<128>
+getShardPathFromFilePath(llvm::SmallString<128> ShardRoot,
+ llvm::StringRef FilePath) {
nit: these micro-optimizations with SmallString s
erichkeane added subscribers: aaron.ballman, erichkeane.
erichkeane added a comment.
You'll not be able to get the C++ spelling without it getting into the
standard. Additionally, it seems easy enough to calculate upon need, so
storing it in the AST is a waste. @aaron.ballman is likely another
I don’t really have much more to add here except to refer you to the style
guide:
https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
Specifically this line: “Use auto if and only if it makes the code more
readable or easier to maintain.”
Given that 2 ou
cfe-commits added a subscriber: aaron.ballman.
cfe-commits added a comment.
I don’t really have much more to add here except to refer you to the style
guide:
https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
Specifically this line: “Use auto if and onl
hokein updated this revision to Diff 173842.
hokein marked an inline comment as done.
hokein added a comment.
update the patch.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53427
Files:
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/Merge.cpp
clangd/index/Se
hokein added inline comments.
Comment at: clangd/index/Index.h:84
inline bool operator==(const SymbolLocation &L, const SymbolLocation &R) {
- return std::tie(L.FileURI, L.Start, L.End) ==
- std::tie(R.FileURI, R.Start, R.End);
+ return std::make_tuple(llvm::StringRef(
gchatelet updated this revision to Diff 173844.
gchatelet added a comment.
- Remove debugging leftover
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53488
Files:
clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
clang-tidy/cppcoreguidelines/NarrowingConversionsChe
Szelethus added a comment.
Hmmm, shouldn't we add this to `MemRegion`'s interface instead?
Repository:
rC Clang
https://reviews.llvm.org/D54466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo
JonasToth accepted this revision.
JonasToth added a comment.
This revision is now accepted and ready to land.
So, finally LGTM :)
Please give @aaron.ballman a chance to comment, as he reviewed too.
Thanks for your patch!
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53488
_
glider created this revision.
glider added reviewers: kcc, rjmccall, rsmith.
Herald added a subscriber: cfe-commits.
This patch adds a new feature, -fsanitize=init-locals, which generates zero
initializers for uninitialized local variables.
There's been discussions in the security community abou
dkrupp updated this revision to Diff 173846.
dkrupp added a comment.
-scanbuild and xcode pictures are included now
-intro text ("What is Static Analysis?" etc.) are put under the Introduction
section
-Download section is created, but I am not sure how well was the this Mac OSX
binary release se
gchatelet added a comment.
In https://reviews.llvm.org/D53488#1296940, @JonasToth wrote:
> So, finally LGTM :)
> Please give @aaron.ballman a chance to comment, as he reviewed too.
>
> Thanks for your patch!
Thank you for bearing with me.
Waiting for input from @aaron.ballman
Repository:
r
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
Comment at: clangd/index/Index.h:85
+ assert(L.FileURI && R.FileURI);
+ int Cmp = std::strcmp(L.FileURI, R.FileURI);
+ return Cmp == 0 && std::tie(L.Start, L.End
lebedev.ri added inline comments.
Comment at: include/clang/Basic/Sanitizers.def:163
+// Initialize local variables.
+SANITIZER("init-locals", InitLocals)
+
Unless i'm mistaken, I suspect you may see some surprising behavior here,
unfortunately.
[[ https://bugs.
grimar added inline comments.
Comment at: lib/Driver/ToolChains/CommonArgs.cpp:813-830
+ if (Arg *A = Args.getLastArg(options::OPT_gsplit_dwarf_EQ))
+if (StringRef(A->getValue()) == "single")
+ return Args.MakeArgString(Output.getFilename());
+
Arg *FinalOutput = Arg
sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ioeric.
Currently, changes *within* CDBs are not tracked (CDB has no facility to do so).
However, discovery of new CDBs are tracked (all files a
Author: krasimir
Date: Tue Nov 13 07:38:12 2018
New Revision: 346756
URL: http://llvm.org/viewvc/llvm-project?rev=346756&view=rev
Log:
[clang-format] Do not treat the asm clobber [ as ObjCExpr
Summary:
The opening square of an inline asm clobber was being annotated as an ObjCExpr.
This caused, am
This revision was automatically updated to reflect the committed changes.
Closed by commit rL346756: [clang-format] Do not treat the asm clobber [ as
ObjCExpr (authored by krasimir, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D54111
Fi
hokein updated this revision to Diff 173856.
hokein marked 3 inline comments as done.
hokein added a comment.
Address review comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53427
Files:
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/Merge.cpp
clangd/i
hokein added inline comments.
Comment at: clangd/index/Index.h:93
+return Cmp < 0;
+ return std::tie(L.Start, L.End) < std::tie(R.Start, R.End);
}
sammccall wrote:
> tie(strcmp(L.FileURI, R.FileURI), ...) < tie(0, ...)?
tie doesn't support this usage, it ex
hubert.reinterpretcast added inline comments.
Comment at: clang/test/CodeGen/altivec-generic-overload.c:74
+ convert1(gv1);
+ // CHECK: call void @_Z8convert1Dv16_a(<16 x i8> %{{[0-9]+}})
+ convert1(gv2);
wuzish wrote:
> hubert.reinterpretcast wrote:
> > Check
Author: erichkeane
Date: Tue Nov 13 07:48:08 2018
New Revision: 346757
URL: http://llvm.org/viewvc/llvm-project?rev=346757&view=rev
Log:
[NFC] Move storage of dispatch-version to GlobalDecl
As suggested by Richard Smith, and initially put up for review here:
https://reviews.llvm.org/D53341, this
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL346757: [NFC] Move storage of dispatch-version to GlobalDecl
(authored by erichkeane, committed by ).
Herald added a subsc
sammccall added inline comments.
Comment at: clangd/index/Index.h:306
+llvm::StringRef S(P);
+CB(S);
+P = S.data();
hokein wrote:
> sammccall wrote:
> > ```assert (!S.data()[S.size()] && "Visited StringRef must be
> > null-terminated")
> Does this as
kadircet updated this revision to Diff 173859.
kadircet marked 13 inline comments as done.
kadircet added a comment.
- Address comments.
- Deleted shard loading logic from backgroundindex.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54269
Files:
clangd/index/Background.cpp
kadircet added inline comments.
Comment at: clangd/index/Background.cpp:52
+llvm::SmallString<128>
+getShardPathFromFilePath(llvm::SmallString<128> ShardRoot,
+ llvm::StringRef FilePath) {
sammccall wrote:
> nit: these micro-optimizations
Quuxplusone marked an inline comment as done.
Quuxplusone added inline comments.
Comment at: docs/LanguageExtensions.rst:1093
library.
+* ``__is_trivially_relocatable`` (Clang): Determines whether moving an object
+ of type ``type``, and then destroying the source object, is
hokein updated this revision to Diff 173861.
hokein marked an inline comment as done.
hokein added a comment.
Add assert.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53427
Files:
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/Merge.cpp
clangd/index/Serializ
hokein added inline comments.
Comment at: clangd/index/Index.h:306
+llvm::StringRef S(P);
+CB(S);
+P = S.data();
sammccall wrote:
> hokein wrote:
> > sammccall wrote:
> > > ```assert (!S.data()[S.size()] && "Visited StringRef must be
> > > null-termi
dblaikie added a comment.
Hey @rjmccall - I'm trying to remember, but can't get it quite clear in my
head. I seem to recall some discussion about trivial_abi not implying/being
strong enough for all the cases that trivial_relocatable sounds like it covers?
Do you happen to remember the distinct
erichkeane added inline comments.
Comment at: docs/LanguageExtensions.rst:1093
library.
+* ``__is_trivially_relocatable`` (Clang): Determines whether moving an object
+ of type ``type``, and then destroying the source object, is functionally
Quuxplusone wrote
ldionne added a comment.
Can this be closed? My understanding is that this has been committed now.
https://reviews.llvm.org/D51762
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
atanasyan added a comment.
In https://reviews.llvm.org/D52418#1294699, @brad wrote:
> Simon?
The testing continues. Unfortunately I do not have access to an appropriate
hardware so cannot control the process. But the task is not abandoned.
Repository:
rC Clang
https://reviews.llvm.org/D52
rjmccall added a comment.
In https://reviews.llvm.org/D50119#1297070, @dblaikie wrote:
> Hey @rjmccall - I'm trying to remember, but can't get it quite clear in my
> head. I seem to recall some discussion about trivial_abi not implying/being
> strong enough for all the cases that trivial_reloca
sammccall added a comment.
A question about the high-level build target setup (I don't know much about XPC
services/frameworks, bear with me...):
This is set up so that the clangd binary (ClangdMain) can run unmodified as an
XPC service, all flags and options are still respected etc.
At the sam
Anastasia updated this revision to Diff 173873.
Anastasia added a comment.
Rewrite how CastKind is set for reference and pointer type.
https://reviews.llvm.org/D53764
Files:
include/clang/Sema/Sema.h
lib/AST/Expr.cpp
lib/CodeGen/CGExpr.cpp
lib/Sema/DeclSpec.cpp
lib/Sema/SemaDecl.cpp
Anastasia added inline comments.
Comment at: lib/Sema/SemaExprCXX.cpp:4289
+ /*BasePath=*/nullptr, CCK)
+ .get();
rjmccall wrote:
> Okay. But if `ToType` *isn't* a reference type, this will never be an
> address-space
Anastasia added a comment.
I am a bit confused about this testing since you are running OpenCL on
architectures that aren't supposed to support it. Can you provide more details
on why you are doing this?
Repository:
rC Clang
https://reviews.llvm.org/D54253
___
Anastasia added a comment.
In https://reviews.llvm.org/D54258#1296706, @richardmembarth wrote:
> CUDA maps `__shared__` internally also to `__attribute__((shared))`:
>
> #define __annotate__(a) \
> __attribute__((a))
> #define __location__(a) \
> __annotate__(a)
> ...
>
rjmccall added inline comments.
Comment at: lib/Sema/SemaExprCXX.cpp:4289
+ /*BasePath=*/nullptr, CCK)
+ .get();
Anastasia wrote:
> rjmccall wrote:
> > Okay. But if `ToType` *isn't* a reference type, this will never be
Author: brunoricci
Date: Tue Nov 13 09:56:44 2018
New Revision: 346770
URL: http://llvm.org/viewvc/llvm-project?rev=346770&view=rev
Log:
[AST][NFC] Pack DeclRefExpr
Move the SourceLocation to the bit-fields of Stmt + clang-format.
This saves one pointer per DeclRefExpr but otherwise NFC.
Modifi
dblaikie accepted this revision.
dblaikie added inline comments.
This revision is now accepted and ready to land.
Comment at: lib/Driver/ToolChains/CommonArgs.cpp:813-830
+ if (Arg *A = Args.getLastArg(options::OPT_gsplit_dwarf_EQ))
+if (StringRef(A->getValue()) == "single")
Quuxplusone marked 3 inline comments as done.
Quuxplusone added inline comments.
Comment at: docs/LanguageExtensions.rst:1096
+ equivalent to copying the underlying bytes and then dropping the source
object
+ on the floor.
* ``__is_destructible`` (MSVC 2013)
dgoldman added a comment.
Since I don't have commit access, @sammccall will land this.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53934
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailma
NoQ accepted this revision.
NoQ added a comment.
Whoops right sry again!
https://reviews.llvm.org/D51762
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
sbenza added a comment.
In https://reviews.llvm.org/D54404#1296224, @aaron.ballman wrote:
> In https://reviews.llvm.org/D54404#1295426, @steveire wrote:
>
> > I acknowledge and share the future-proofing concern.
> >
> > We could possibly use something trait-based instead and put the trait
> > be
NoQ added a comment.
Ugh i cannot close it :/
https://reviews.llvm.org/D51762
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
void added a comment.
Ping?
Repository:
rC Clang
https://reviews.llvm.org/D54356
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
void added a comment.
Ping?
Repository:
rC Clang
https://reviews.llvm.org/D54355
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
vsapsai added a comment.
Ping.
https://reviews.llvm.org/D54245
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rjmccall added inline comments.
Comment at: docs/LanguageExtensions.rst:1096
+ equivalent to copying the underlying bytes and then dropping the source
object
+ on the floor.
* ``__is_destructible`` (MSVC 2013)
Quuxplusone wrote:
> @rjmccall wrote:
> > trivial
Quuxplusone added inline comments.
Comment at: docs/LanguageExtensions.rst:1096
+ equivalent to copying the underlying bytes and then dropping the source
object
+ on the floor.
* ``__is_destructible`` (MSVC 2013)
rjmccall wrote:
> Quuxplusone wrote:
> > @rjmc
erichkeane added inline comments.
Comment at: lib/AST/Type.cpp:2234
+bool QualType::isTriviallyRelocatableType(const ASTContext &Context) const {
+ QualType T = Context.getBaseElementType(*this);
+ if (T->isIncompleteType())
Quuxplusone wrote:
> erichkeane wrot
Author: brunoricci
Date: Tue Nov 13 11:27:39 2018
New Revision: 346781
URL: http://llvm.org/viewvc/llvm-project?rev=346781&view=rev
Log:
[AST][NFC] Style fixes for UnaryOperator
In preparation for the patch which will move some data to the bit-fields
of Stmt. In particular, rename the private var
rjmccall added inline comments.
Comment at: docs/LanguageExtensions.rst:1096
+ equivalent to copying the underlying bytes and then dropping the source
object
+ on the floor.
* ``__is_destructible`` (MSVC 2013)
Quuxplusone wrote:
> rjmccall wrote:
> > Quuxplus
NoQ added a comment.
In https://reviews.llvm.org/D54438#1296832, @Szelethus wrote:
> - Some checkers do not register themselves in all cases[1], which should
> probably be handled at a higher level, preferably in `Checkers.td`. Warnings
> could be emitted when a checker is explicitly enabled bu
vsapsai added a comment.
The test case I've promised is
touch a.h
touch b.h
touch c.h
ln -s b.h d.h
echo '#include "a.h"' > umbrella.h
echo '#include "b.h"' >> umbrella.h
echo '#include "b.h"' > test_case.c
cat < module.modulemap
module my_module {
umbrella header "
NoQ created this revision.
NoQ added reviewers: george.karpenkov, rsmith.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus,
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Use the new fancy method introduced in https://reviews.llvm.org/D54486 to
simplify so
scott.linder created this revision.
Herald added a subscriber: cfe-commits.
See the parent revision https://reviews.llvm.org/D54487 for differences between
this implementation and the equivalent GCC option.
Repository:
rC Clang
https://reviews.llvm.org/D54489
Files:
docs/ClangCommandLineR
Author: dblaikie
Date: Tue Nov 13 12:08:13 2018
New Revision: 346789
URL: http://llvm.org/viewvc/llvm-project?rev=346789&view=rev
Log:
DebugInfo: Add a driver flag for DWARF debug_ranges base address specifier use.
Summary:
This saves a lot of relocations in optimized object files (at the cost
of
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL346789: DebugInfo: Add a driver flag for DWARF debug_ranges
base address specifier use. (authored by dblaikie, committed b
dblaikie added a comment.
Settled on renaming the flag for consistency with, say, -fdebug-types-section,
to -fdebug-ranges-base-address (though 'debug' is sort of ambiguous (Clang can
produce non-DWARF debug info) but this driver (as opposed to clang-cl) is
intended for DWARF emission, not PDB
kcc added a comment.
This new flag inhibits the warnings from -Wuninitialized, right?
While this is fine for experimenting (and I want to have this in ToT to enable
wide experimentation)
we should clearly state (in the comments) that the final intent is to make the
feature work together with -W
rsmith added a comment.
Can you explain more about the justification for this? The code today has a
covered switch, which is useful for maintainability -- anyone adding a new
`Expr` node gets told they need to think about and update this code. Are there
any cases where we check for an ICE and a
Author: brunoricci
Date: Tue Nov 13 12:23:11 2018
New Revision: 346793
URL: http://llvm.org/viewvc/llvm-project?rev=346793&view=rev
Log:
[AST][NFC] Order the bit-field classes of Stmt like in StmtNodes.td
Reorder the bit-field classes and the members of the anonymous union
so that they both match
rsmith added inline comments.
Comment at: include/clang/AST/Expr.h:908-912
+ static ConstantExpr *Create(const ASTContext &Context, Expr *E) {
+if (ConstantExpr *CE = dyn_cast(E))
+ return CE;
+return new (Context) ConstantExpr(E);
+ }
If we're cre
troyj added a comment.
I realize that you're probably striving for option compatibility with gcc, but
continuing to name it -frecord-gcc-switches when it actually records Clang
switches seems weird to me. It almost sounds like something that would dump
gcc equivalents of all Clang options, or
void added a comment.
This code is called in over 90 places, so it's hard to tell if they all are in
a constant context. Though I suppose that what this code is supposed to check
for would work the same in a constant context as without one. I can revert this
if you want, but to be honest the or
scott.linder added a comment.
In https://reviews.llvm.org/D54489#1297504, @troyj wrote:
> I realize that you're probably striving for option compatibility with gcc,
> but continuing to name it -frecord-gcc-switches when it actually records
> Clang switches seems weird to me. It almost sounds l
rsmith added a comment.
In https://reviews.llvm.org/D54356#1297470, @rsmith wrote:
> Can you explain more about the justification for this? The code today has a
> covered switch, which is useful for maintainability -- anyone adding a new
> `Expr` node gets told they need to think about and upda
void added a comment.
Okay. I'll revert this then.
Repository:
rC Clang
https://reviews.llvm.org/D54356
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
In https://reviews.llvm.org/D54356#1297506, @void wrote:
> This code is called in over 90 places, so it's hard to tell if they all are
> in a constant context. Though I suppose that what this
rsmith added a comment.
In https://reviews.llvm.org/D54356#1297522, @void wrote:
> Okay. I'll revert this then.
I don't think we necessarily need the change in the other patch that's based on
this one, but I still think this refactoring is an improvement :)
Repository:
rC Clang
https://re
void abandoned this revision.
void added a comment.
This isn't necessary. We can assume it's in a constant context because it's
checking for an ICE.
Repository:
rC Clang
https://reviews.llvm.org/D54356
___
cfe-commits mailing list
cfe-commits@li
void added a comment.
In https://reviews.llvm.org/D54356#1297543, @rsmith wrote:
> In https://reviews.llvm.org/D54356#1297522, @void wrote:
>
> > Okay. I'll revert this then.
>
>
> I don't think we necessarily need the change in the other patch that's based
> on this one, but I still think this
1 - 100 of 158 matches
Mail list logo
